Skip to content

AMDGPU: Remove incorrect uses of SubtaretPredicate around DS_Reals #85001

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 3 commits into from
Mar 13, 2024
Merged

AMDGPU: Remove incorrect uses of SubtaretPredicate around DS_Reals #85001

merged 3 commits into from
Mar 13, 2024

Conversation

changpeng
Copy link
Contributor

@changpeng changpeng commented Mar 13, 2024

SubtargetPredicate is copied from DS_Pseudo to DS_Real. We should not use another SubtargetPredicate assignment around DS_Real, because doing so will override the predicate from DS_Pseudo.

For example, for DS_ADD_RTN_F64, SubtargetPredicate was set to HasLdsAtomicAddF64 in Pseudo. And it will be overridden to isGFX90APlus if we assign isGFX90APlus to SubtargetPredicate in Real definition.

  SubtargetPredicate is copied from DS_Pseudo to DS_Real. We should
not use another SubtargetPredicate around DS_Real, because doing this
will override the predicate from DS_Pseudo.

  For examle, for DS_ADD_RTN_F64, SubtargetPredicate was set to
HasLdsAtomicAddF64 in Pseudo. And it will be overriden to isGFX90APlus
if we assign isGFX90APlus to SubtargetPredicate in Real definition.
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Changpeng Fang (changpeng)

Changes

SubtargetPredicate is copied from DS_Pseudo to DS_Real. We should not use another SubtargetPredicate around DS_Real, because doing this will override the predicate from DS_Pseudo.

For examle, for DS_ADD_RTN_F64, SubtargetPredicate was set to HasLdsAtomicAddF64 in Pseudo. And it will be overriden to isGFX90APlus if we assign isGFX90APlus to SubtargetPredicate in Real definition.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/DSInstructions.td (+9-11)
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 87ace01a6d0e56..b869a1a12f5d29 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -1735,14 +1735,12 @@ def DS_WRITE_B128_vi      : DS_Real_vi<0xdf, DS_WRITE_B128>;
 def DS_READ_B96_vi        : DS_Real_vi<0xfe, DS_READ_B96>;
 def DS_READ_B128_vi       : DS_Real_vi<0xff, DS_READ_B128>;
 
-let SubtargetPredicate = isGFX90APlus in {
-  def DS_ADD_F64_vi     : DS_Real_vi<0x5c, DS_ADD_F64>;
-  def DS_ADD_RTN_F64_vi : DS_Real_vi<0x7c, DS_ADD_RTN_F64>;
-} // End SubtargetPredicate = isGFX90APlus
-
-let SubtargetPredicate = isGFX940Plus in {
-  def DS_PK_ADD_F16_vi     : DS_Real_vi<0x17, DS_PK_ADD_F16>;
-  def DS_PK_ADD_RTN_F16_vi : DS_Real_vi<0xb7, DS_PK_ADD_RTN_F16>;
-  def DS_PK_ADD_BF16_vi     : DS_Real_vi<0x18, DS_PK_ADD_BF16>;
-  def DS_PK_ADD_RTN_BF16_vi : DS_Real_vi<0xb8, DS_PK_ADD_RTN_BF16>;
-} // End SubtargetPredicate = isGFX940Plus
+// GFX90A
+def DS_ADD_F64_vi     : DS_Real_vi<0x5c, DS_ADD_F64>;
+def DS_ADD_RTN_F64_vi : DS_Real_vi<0x7c, DS_ADD_RTN_F64>;
+
+// GFX940
+def DS_PK_ADD_F16_vi     : DS_Real_vi<0x17, DS_PK_ADD_F16>;
+def DS_PK_ADD_RTN_F16_vi : DS_Real_vi<0xb7, DS_PK_ADD_RTN_F16>;
+def DS_PK_ADD_BF16_vi     : DS_Real_vi<0x18, DS_PK_ADD_BF16>;
+def DS_PK_ADD_RTN_BF16_vi : DS_Real_vi<0xb8, DS_PK_ADD_RTN_BF16>;

@changpeng changpeng requested review from jayfoad and srpande March 13, 2024 00:24
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Could have used +

@changpeng changpeng merged commit e703c73 into llvm:main Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants