Skip to content

[AMDGPU] Add new aliases ds_subrev_u32/u64 for ds_rsub_u32/u64 #83118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Feb 27, 2024

Note that the instructions have not been renamed and that there are no
corresponding aliases for ds_rsub_rtn_u32/u64. This matches SP3
behavior.

Note that the instructions have not been renamed and that there are no
corresponding aliases for ds_rsub_rtn_u32/u64. This matches SP3
behavior.
@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Feb 27, 2024
@jayfoad jayfoad requested a review from mbrkusanin February 27, 2024 10:07
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2024

@llvm/pr-subscribers-mc

Author: Jay Foad (jayfoad)

Changes

Note that the instructions have not been renamed and that there are no
corresponding aliases for ds_rsub_rtn_u32/u64. This matches SP3
behavior.


Full diff: https://github.com/llvm/llvm-project/pull/83118.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/DSInstructions.td (+4)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s (+6)
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 515b9476b25b75..074e13317ef89d 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -1259,6 +1259,10 @@ defm DS_PK_ADD_RTN_F16    : DS_Real_gfx12<0x0aa>;
 defm DS_PK_ADD_BF16       : DS_Real_gfx12<0x09b>;
 defm DS_PK_ADD_RTN_BF16   : DS_Real_gfx12<0x0ab>;
 
+// New aliases added in GFX12 without renaming the instructions.
+def : MnemonicAlias<"ds_subrev_u32", "ds_rsub_u32">, Requires<[isGFX12Plus]>;
+def : MnemonicAlias<"ds_subrev_u64", "ds_rsub_u64">, Requires<[isGFX12Plus]>;
+
 //===----------------------------------------------------------------------===//
 // GFX11.
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s b/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
index aa4b028160175c..8b076094aa8581 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
@@ -23,3 +23,9 @@ ds_min_rtn_f32 v5, v1, v2
 
 ds_min_rtn_f64 v[5:6], v1, v[2:3]
 // GFX12: [0x00,0x00,0xc8,0xd9,0x01,0x02,0x00,0x05]
+
+ds_subrev_u32 v1, v2
+// GFX12: [0x00,0x00,0x08,0xd8,0x01,0x02,0x00,0x00]
+
+ds_subrev_u64 v1, v[2:3]
+// GFX12: [0x00,0x00,0x08,0xd9,0x01,0x02,0x00,0x00]

@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

Changes

Note that the instructions have not been renamed and that there are no
corresponding aliases for ds_rsub_rtn_u32/u64. This matches SP3
behavior.


Full diff: https://github.com/llvm/llvm-project/pull/83118.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/DSInstructions.td (+4)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s (+6)
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 515b9476b25b75..074e13317ef89d 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -1259,6 +1259,10 @@ defm DS_PK_ADD_RTN_F16    : DS_Real_gfx12<0x0aa>;
 defm DS_PK_ADD_BF16       : DS_Real_gfx12<0x09b>;
 defm DS_PK_ADD_RTN_BF16   : DS_Real_gfx12<0x0ab>;
 
+// New aliases added in GFX12 without renaming the instructions.
+def : MnemonicAlias<"ds_subrev_u32", "ds_rsub_u32">, Requires<[isGFX12Plus]>;
+def : MnemonicAlias<"ds_subrev_u64", "ds_rsub_u64">, Requires<[isGFX12Plus]>;
+
 //===----------------------------------------------------------------------===//
 // GFX11.
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s b/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
index aa4b028160175c..8b076094aa8581 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_ds_alias.s
@@ -23,3 +23,9 @@ ds_min_rtn_f32 v5, v1, v2
 
 ds_min_rtn_f64 v[5:6], v1, v[2:3]
 // GFX12: [0x00,0x00,0xc8,0xd9,0x01,0x02,0x00,0x05]
+
+ds_subrev_u32 v1, v2
+// GFX12: [0x00,0x00,0x08,0xd8,0x01,0x02,0x00,0x00]
+
+ds_subrev_u64 v1, v[2:3]
+// GFX12: [0x00,0x00,0x08,0xd9,0x01,0x02,0x00,0x00]

// GFX12: [0x00,0x00,0x08,0xd8,0x01,0x02,0x00,0x00]

ds_subrev_u64 v1, v[2:3]
// GFX12: [0x00,0x00,0x08,0xd9,0x01,0x02,0x00,0x00]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these alias tests it makes sense to me to include full output in CHECK lines not just the hex values. That way we can see the other name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Done. I will go back and fix the existing tests separately.

@jayfoad jayfoad merged commit ca0560d into llvm:main Feb 27, 2024
@jayfoad jayfoad deleted the ds-subrev branch February 27, 2024 10:58
jayfoad added a commit to jayfoad/llvm-project that referenced this pull request Feb 29, 2024
Following on from llvm#83118, this adds aliases for the "rtn" forms of these
instructions. The fact that they were missing from SP3 was an oversight
which has been fixed now.
jayfoad added a commit that referenced this pull request Feb 29, 2024
#83408)

Following on from #83118, this adds aliases for the "rtn" forms of these
instructions. The fact that they were missing from SP3 was an oversight
which has been fixed now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants