AMDGPU: Fix introducing use of killed vgpr in gfx908 agpr copy

[arsenm]

When searching for an existing VGPR source for an AGPR to AGPR
copy on gfx908, this wasn't verifying the vgpr wasn't killed by
other prior uses.

[arsenm]

• AMDGPU: Add pass to replace constant materialize with AV pseudos #149292
• AMDGPU: Fix introducing use of killed vgpr in gfx908 agpr copy #149291 👈 (View in Graphite)
• AMDGPU: Always use AV spill pseudos on targets with AGPRs #149099
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

When searching for an existing VGPR source for an AGPR to AGPR
copy on gfx908, this wasn't verifying the vgpr wasn't killed by
other prior uses.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+2-1)
• (modified) llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir (+83)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index b1116974642c9..c8935f0cb6034 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -687,7 +687,8 @@ static void indirectCopyToAGPR(const SIInstrInfo &TII,
         if (!SafeToPropagate)
           break;
 
-        DefOp.setIsKill(false);
+        for (auto I = Def; I != MI; ++I)
+          I->clearRegisterKills(DefOp.getReg(), &RI);
       }
 
       MachineInstrBuilder Builder =
diff --git a/llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir b/llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
index 2bd1b8bf3f3f6..d22a4b978980f 100644
--- a/llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
+++ b/llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
@@ -45,6 +45,9 @@
     define amdgpu_kernel void @copy_agpr_to_agpr_tuple() #0 { ret void }
     define amdgpu_kernel void @copy_agpr_to_agpr_tuple_kill() #0 { ret void }
 
+    define amdgpu_kernel void @look_for_vgpr_killed() #0 { ret void }
+    define amdgpu_kernel void @look_for_vgpr_killed_tuple() #0 { ret void }
+
     attributes #0 = { "amdgpu-flat-work-group-size"="1,256" }
 ...
 
@@ -1517,3 +1520,83 @@ body:             |
     renamable $agpr4_agpr5_agpr6_agpr7 = COPY renamable killed $agpr0_agpr1_agpr2_agpr3, implicit $exec
     S_ENDPGM 0, implicit $agpr4_agpr5_agpr6_agpr7
 ...
+
+# Make sure the expansion of the a-to-a copy doesn't introduce a use
+# after kill of the source vgpr
+---
+name: look_for_vgpr_killed
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $agpr0
+
+    ; GFX908-LABEL: name: look_for_vgpr_killed
+    ; GFX908: liveins: $agpr0
+    ; GFX908-NEXT: {{  $}}
+    ; GFX908-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    ; GFX908-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX908-NEXT: S_NOP 0, implicit $vgpr0
+    ; GFX908-NEXT: $agpr1 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ;
+    ; GFX90A-LABEL: name: look_for_vgpr_killed
+    ; GFX90A: liveins: $agpr0
+    ; GFX90A-NEXT: {{  $}}
+    ; GFX90A-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    ; GFX90A-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX90A-NEXT: S_NOP 0, implicit killed $vgpr0
+    ; GFX90A-NEXT: $agpr1 = V_ACCVGPR_MOV_B32 $agpr0, implicit $exec
+    ;
+    ; GFX942-LABEL: name: look_for_vgpr_killed
+    ; GFX942: liveins: $agpr0
+    ; GFX942-NEXT: {{  $}}
+    ; GFX942-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    ; GFX942-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX942-NEXT: S_NOP 0, implicit killed $vgpr0
+    ; GFX942-NEXT: $agpr1 = V_ACCVGPR_MOV_B32 $agpr0, implicit $exec
+    $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    $agpr0 = COPY $vgpr0
+    S_NOP 0, implicit killed $vgpr0
+    $agpr1 = COPY $agpr0
+
+...
+
+---
+name: look_for_vgpr_killed_tuple
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $agpr0
+
+    ; GFX908-LABEL: name: look_for_vgpr_killed_tuple
+    ; GFX908: liveins: $agpr0
+    ; GFX908-NEXT: {{  $}}
+    ; GFX908-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec, implicit-def $vgpr0_vgpr1
+    ; GFX908-NEXT: $vgpr1 = V_MOV_B32_e32 1, implicit $exec
+    ; GFX908-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX908-NEXT: S_NOP 0, implicit $vgpr0_vgpr1
+    ; GFX908-NEXT: $agpr1 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ;
+    ; GFX90A-LABEL: name: look_for_vgpr_killed_tuple
+    ; GFX90A: liveins: $agpr0
+    ; GFX90A-NEXT: {{  $}}
+    ; GFX90A-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec, implicit-def $vgpr0_vgpr1
+    ; GFX90A-NEXT: $vgpr1 = V_MOV_B32_e32 1, implicit $exec
+    ; GFX90A-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX90A-NEXT: S_NOP 0, implicit killed $vgpr0_vgpr1
+    ; GFX90A-NEXT: $agpr1 = V_ACCVGPR_MOV_B32 $agpr0, implicit $exec
+    ;
+    ; GFX942-LABEL: name: look_for_vgpr_killed_tuple
+    ; GFX942: liveins: $agpr0
+    ; GFX942-NEXT: {{  $}}
+    ; GFX942-NEXT: $vgpr0 = V_MOV_B32_e32 0, implicit $exec, implicit-def $vgpr0_vgpr1
+    ; GFX942-NEXT: $vgpr1 = V_MOV_B32_e32 1, implicit $exec
+    ; GFX942-NEXT: $agpr0 = V_ACCVGPR_WRITE_B32_e64 $vgpr0, implicit $exec
+    ; GFX942-NEXT: S_NOP 0, implicit killed $vgpr0_vgpr1
+    ; GFX942-NEXT: $agpr1 = V_ACCVGPR_MOV_B32 $agpr0, implicit $exec
+    $vgpr0 = V_MOV_B32_e32 0, implicit $exec, implicit-def $vgpr0_vgpr1
+    $vgpr1 = V_MOV_B32_e32 1, implicit $exec
+    $agpr0 = COPY $vgpr0
+    S_NOP 0, implicit killed $vgpr0_vgpr1
+    $agpr1 = COPY $agpr0
+
+...

[arsenm]

Merge activity

• Jul 18, 6:27 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 18, 6:32 AM UTC: Graphite rebased this pull request as part of a merge.
• Jul 18, 6:34 AM UTC: @arsenm merged this pull request with Graphite.