Skip to content
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

[AMDGPU] Fix some MIR tests #66090

Merged
merged 1 commit into from
Sep 12, 2023
Merged

[AMDGPU] Fix some MIR tests #66090

merged 1 commit into from
Sep 12, 2023

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Sep 12, 2023

Fix some problems in hand written MIR tests that only showed up when I
tried to run LiveIntervals on them, after which they failed machine
verification with "Use not jointly dominated by defs" errors.

Fix some problems in hand written MIR tests that only showed up when I
tried to run LiveIntervals on them, after which they failed machine
verification with "Use not jointly dominated by defs" errors.
@jayfoad jayfoad requested a review from a team as a code owner September 12, 2023 14:11
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 12, 2023

@llvm/pr-subscribers-backend-amdgpu

Changes

Fix some problems in hand written MIR tests that only showed up when I
tried to run LiveIntervals on them, after which they failed machine
verification with "Use not jointly dominated by defs" errors.

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

2 Files Affected:

  • (modified) llvm/test/CodeGen/AMDGPU/twoaddr-fma-f64.mir (+6-2)
  • (modified) llvm/test/CodeGen/AMDGPU/twoaddr-mad.mir (+12-12)
diff --git a/llvm/test/CodeGen/AMDGPU/twoaddr-fma-f64.mir b/llvm/test/CodeGen/AMDGPU/twoaddr-fma-f64.mir
index 0d1dcd3060a0af1..db5cf285c109718 100644
--- a/llvm/test/CodeGen/AMDGPU/twoaddr-fma-f64.mir
+++ b/llvm/test/CodeGen/AMDGPU/twoaddr-fma-f64.mir
@@ -56,7 +56,7 @@ body:             |
 ...
 
 # GCN-LABEL: name: test_fmaak_sgpr_src0_f64
-# GCN: V_FMA_F64_e64 0, killed %0, 0, %1, 0, %2:vreg_64_align2, 0, 0, implicit $mode, implicit $exec
+# GCN: V_FMA_F64_e64 0, killed %0, 0, %1, 0, %2, 0, 0, implicit $mode, implicit $exec
 
 ---
 name:            test_fmaak_sgpr_src0_f64
@@ -70,12 +70,13 @@ body:             |
 
     %0 = IMPLICIT_DEF
     %1 = V_MOV_B64_PSEUDO 4607182418800017408, implicit $exec
+    %2 = IMPLICIT_DEF
     %3 = V_FMAC_F64_e32 killed %0, %1, %2, implicit $mode, implicit $exec
 
 ...
 
 # GCN-LABEL: name: test_fmaak_inlineimm_src0_f64
-# GCN: V_FMA_F64_e64 0, 4611686018427387904, 0, %0, 0, %1:vreg_64_align2, 0, 0, implicit $mode, implicit $exec
+# GCN: V_FMA_F64_e64 0, 4611686018427387904, 0, %0, 0, %1, 0, 0, implicit $mode, implicit $exec
 
 ---
 name:            test_fmaak_inlineimm_src0_f64
@@ -87,6 +88,7 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B64_PSEUDO 4607182418800017408, implicit $exec
+    %1 = IMPLICIT_DEF
     %2 = V_FMAC_F64_e32 4611686018427387904, %0, %1, implicit $mode, implicit $exec
 
 ...
@@ -104,6 +106,7 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B64_PSEUDO 4607182418800017408, implicit $exec
+    %1 = IMPLICIT_DEF
     %2 = V_FMAC_F64_e32 4636737291354636288, %0, %1, implicit $mode, implicit $exec
 
 ...
@@ -124,6 +127,7 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B64_PSEUDO 4607182418800017408, implicit $exec
+    %1 = IMPLICIT_DEF
     %2 = V_FMAC_F64_e32 %stack.0, %0, %1, implicit $mode, implicit $exec
 
 ...
diff --git a/llvm/test/CodeGen/AMDGPU/twoaddr-mad.mir b/llvm/test/CodeGen/AMDGPU/twoaddr-mad.mir
index 4e916ae93ada738..1458e8135ef2da3 100644
--- a/llvm/test/CodeGen/AMDGPU/twoaddr-mad.mir
+++ b/llvm/test/CodeGen/AMDGPU/twoaddr-mad.mir
@@ -113,7 +113,7 @@ body:             |
 
 # GCN-LABEL: name: test_madak_sgpr_src0_f32
 # GCN: %1:vgpr_32 = V_MOV_B32_e32 1078523331, implicit $exec
-# GCN: %2:vgpr_32 = V_MAD_F32_e64 0, killed %0, 0, %1, 0, %3:vgpr_32, 0, 0, implicit $mode, implicit $exec
+# GCN: %3:vgpr_32 = V_MAD_F32_e64 0, killed %0, 0, %1, 0, %2, 0, 0, implicit $mode, implicit $exec
 
 ---
 name:            test_madak_sgpr_src0_f32
@@ -127,14 +127,15 @@ body:             |
 
     %0 = IMPLICIT_DEF
     %1 = V_MOV_B32_e32 1078523331, implicit $exec
-    %2 = V_MAC_F32_e32 killed %0, %1, %3, implicit $mode, implicit $exec
+    %2 = IMPLICIT_DEF
+    %3 = V_MAC_F32_e32 killed %0, %1, %2, implicit $mode, implicit $exec
 
 ...
 
 # This can still fold if this is an inline immediate.
 
 # GCN-LABEL: name: test_madak_inlineimm_src0_f32
-# GCN: %1:vgpr_32 = V_MADMK_F32 1073741824, 1078523331, %2:vgpr_32, implicit $mode, implicit $exec
+# GCN: %2:vgpr_32 = V_MADMK_F32 1073741824, 1078523331, %1, implicit $mode, implicit $exec
 
 ---
 name:            test_madak_inlineimm_src0_f32
@@ -146,13 +147,14 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B32_e32 1078523331, implicit $exec
-    %1 = V_MAC_F32_e32 1073741824, %0, %2, implicit $mode, implicit $exec
+    %1 = IMPLICIT_DEF
+    %2 = V_MAC_F32_e32 1073741824, %0, %1, implicit $mode, implicit $exec
 
 ...
 # Non-inline immediate uses constant bus already.
 
 # GCN-LABEL: name: test_madak_otherimm_src0_f32
-# GCN: %1:vgpr_32 = V_MADMK_F32 %0, 1120403456, %2:vgpr_32, implicit $mode, implicit $exec
+# GCN: %2:vgpr_32 = V_MADMK_F32 %0, 1120403456, %1, implicit $mode, implicit $exec
 
 ---
 name:            test_madak_otherimm_src0_f32
@@ -164,13 +166,14 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B32_e32 1078523331, implicit $exec
-    %1 = V_MAC_F32_e32 1120403456, %0, %2, implicit $mode, implicit $exec
+    %1 = IMPLICIT_DEF
+    %2 = V_MAC_F32_e32 1120403456, %0, %1, implicit $mode, implicit $exec
 
 ...
 # Non-inline immediate uses constant bus already.
 
 # GCN-LABEL: name: test_madak_other_constantlike_src0_f32
-# GCN: %1:vgpr_32 = V_MAC_F32_e32 %stack.0, %0, %1, implicit $mode, implicit $exec
+# GCN: %2:vgpr_32 = V_MAC_F32_e32 %stack.0, %0, %2, implicit $mode, implicit $exec
 ---
 name:            test_madak_other_constantlike_src0_f32
 registers:
@@ -185,7 +188,8 @@ body:             |
   bb.0:
 
     %0 = V_MOV_B32_e32 1078523331, implicit $exec
-    %1 = V_MAC_F32_e32 %stack.0, %0, %2, implicit $mode, implicit $exec
+    %1 = IMPLICIT_DEF
+    %2 = V_MAC_F32_e32 %stack.0, %0, %1, implicit $mode, implicit $exec
 
 ...
 
@@ -194,12 +198,8 @@ body:             |
 
 ---
 name:            test_madak_inline_literal_f16
-liveins:
-  - { reg: '$vgpr0', virtual-reg: '%3' }
 body:             |
   bb.0:
-    liveins: $vgpr0
-
     %3:vgpr_32 = COPY killed $vgpr0
 
     %26:vgpr_32 = V_MOV_B32_e32 49664, implicit $exec

@@ -185,7 +188,8 @@ body: |
bb.0:

%0 = V_MOV_B32_e32 1078523331, implicit $exec
%1 = V_MAC_F32_e32 %stack.0, %0, %2, implicit $mode, implicit $exec
%1 = IMPLICIT_DEF
Copy link
Contributor

Choose a reason for hiding this comment

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

IMPLICIT_DEF isn't really representative of running twoaddrs, which runs after ProcessImplicitDefs. Better to have an undef operand

@@ -127,14 +127,15 @@ body: |

%0 = IMPLICIT_DEF
%1 = V_MOV_B32_e32 1078523331, implicit $exec
%2 = V_MAC_F32_e32 killed %0, %1, %3, implicit $mode, implicit $exec
%2 = IMPLICIT_DEF
%3 = V_MAC_F32_e32 killed %0, %1, %2, implicit $mode, implicit $exec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMPLICIT_DEF isn't really representative of running twoaddrs, which runs after ProcessImplicitDefs. Better to have an undef operand

I don't want to do that because changing the third operand in this case to be undef changes the behaviour of twoaddrs - it now tries to convert this MAC to a MAD, and the same for a bunch of other cases.

I could use a real instruction instead of IMPLICIT_DEF if that's preferable?

@jayfoad jayfoad merged commit 928c9d6 into llvm:main Sep 12, 2023
2 of 3 checks passed
@jayfoad jayfoad deleted the mir-fixes branch September 12, 2023 15:32
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Fix some problems in hand written MIR tests that only showed up when I
tried to run LiveIntervals on them, after which they failed machine
verification with "Use not jointly dominated by defs" errors.
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.

None yet

3 participants