[llvm] 592f2e8 - AMDGPU: Fix verifier error on spilling partially defined SGPRs

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2020-07-28T20:01:57-04:00
New Revision: 592f2e8d1ceb8ecec6f8b54eeb9fd7e0a099c0fe

URL: https://github.com/llvm/llvm-project/commit/592f2e8d1ceb8ecec6f8b54eeb9fd7e0a099c0fe
DIFF: https://github.com/llvm/llvm-project/commit/592f2e8d1ceb8ecec6f8b54eeb9fd7e0a099c0fe.diff

LOG: AMDGPU: Fix verifier error on spilling partially defined SGPRs

This needs an implicit def of the super-register in case one of the
lanes isn't defined, similar to copyPhysReg (or the not-VGPR spill
case below). This showed up in GlobalISel testing since it currently
doesn't fold out many undef instructions.

Added: 
    llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir

Modified: 
    llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
index 956658296a94..9263be6e1abf 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
@@ -1029,13 +1029,19 @@ bool SIRegisterInfo::spillSGPR(MachineBasicBlock::iterator MI,
 
       // Mark the "old value of vgpr" input undef only if this is the first sgpr
       // spill to this specific vgpr in the first basic block.
-      BuildMI(*MBB, MI, DL,
+      auto MIB = BuildMI(*MBB, MI, DL,
               TII->getMCOpcodeFromPseudo(AMDGPU::V_WRITELANE_B32),
               Spill.VGPR)
         .addReg(SubReg, getKillRegState(IsKill))
         .addImm(Spill.Lane)
         .addReg(Spill.VGPR, VGPRDefined ? 0 : RegState::Undef);
 
+      if (i == 0 && NumSubRegs > 1) {
+        // We may be spilling a super-register which is only partially defined,
+        // and need to ensure later spills think the value is defined.
+        MIB.addReg(SuperReg, RegState::ImplicitDefine);
+      }
+
       // FIXME: Since this spills to another register instead of an actual
       // frame index, we should delete the frame index when all references to
       // it are fixed.

diff  --git a/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir b/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir
new file mode 100644
index 000000000000..777a692affca
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/sgpr-spill-partially-undef.mir
@@ -0,0 +1,52 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs -run-pass=si-lower-sgpr-spills -o - %s  | FileCheck %s
+#
+# Check for liveness errors when spilling partially defined super registers.
+
+---
+name:  sgpr_spill_s64_undef_high32
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  hasSpilledSGPRs:     true
+  scratchRSrcReg:  '$sgpr96_sgpr97_sgpr98_sgpr99'
+  stackPtrOffsetReg: '$sgpr32'
+
+stack:
+  - { id: 0, type: spill-slot, size: 8, alignment: 4, stack-id: sgpr-spill }
+
+body:             |
+  bb.0:
+    liveins: $sgpr4
+
+    ; CHECK-LABEL: name: sgpr_spill_s64_undef_high32
+    ; CHECK: liveins: $sgpr4, $vgpr0
+    ; CHECK: $vgpr0 = V_WRITELANE_B32_gfx6_gfx7 $sgpr4, 0, undef $vgpr0, implicit-def $sgpr4_sgpr5
+    ; CHECK: $vgpr0 = V_WRITELANE_B32_gfx6_gfx7 $sgpr5, 1, $vgpr0
+    SI_SPILL_S64_SAVE renamable $sgpr4_sgpr5, %stack.0, implicit $exec, implicit $sgpr96_sgpr97_sgpr98_sgpr99, implicit $sgpr32 :: (store 8 into %stack.0, align 4, addrspace 5)
+
+...
+
+---
+name:  sgpr_spill_s64_undef_low32
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  hasSpilledSGPRs:     true
+  scratchRSrcReg:  '$sgpr96_sgpr97_sgpr98_sgpr99'
+  stackPtrOffsetReg: '$sgpr32'
+
+stack:
+  - { id: 0, type: spill-slot, size: 8, alignment: 4, stack-id: sgpr-spill }
+
+body:             |
+  bb.0:
+    liveins: $sgpr5
+
+    ; CHECK-LABEL: name: sgpr_spill_s64_undef_low32
+    ; CHECK: liveins: $sgpr5, $vgpr0
+    ; CHECK: $vgpr0 = V_WRITELANE_B32_gfx6_gfx7 $sgpr4, 0, undef $vgpr0, implicit-def $sgpr4_sgpr5
+    ; CHECK: $vgpr0 = V_WRITELANE_B32_gfx6_gfx7 $sgpr5, 1, $vgpr0
+    SI_SPILL_S64_SAVE renamable $sgpr4_sgpr5, %stack.0, implicit $exec, implicit $sgpr96_sgpr97_sgpr98_sgpr99, implicit $sgpr32 :: (store 8 into %stack.0, align 4, addrspace 5)
+
+...