AMDGPU: Fix verifier error from partially undef copy

[arsenm]

Summary

In this situation:

%VGPR2<def> = BUFFER_LOAD_DWORD_OFFSET %SGPR8_SGPR9_SGPR10_SGPR11,
%VGPR7<def,tied3> = V_MAC_F32_e32 %VGPR0<undef>, %VGPR1<kill>, %VGPR7<kill,tied0>, %EXEC<imp-use>
%VGPR3_VGPR4_VGPR5_VGPR6<def> = COPY %VGPR0_VGPR1_VGPR2_VGPR3
%VGPR4<def> = COPY %VGPR2

The copy for VGPR1 -> VGPR4 was an error from reading undefined VGPR1,
but VGPR4 is defined immediately after this copy.

[MatzeB]

Yep that's the current way to indicate to the verifier that you are really copying a bigger register.

[arsenm]

r275635

lib/Target/AMDGPU/SIInstrInfo.cpp

void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
   };
 
   static const int16_t Sub0_1[] = {
     AMDGPU::sub0, AMDGPU::sub1,
   };
 
   unsigned Opcode;
   ArrayRef<int16_t> SubIndices;
-  bool Forward;
 
   if (AMDGPU::SReg_32RegClass.contains(DestReg)) {
     assert(AMDGPU::SReg_32RegClass.contains(SrcReg));
     BuildMI(MBB, MI, DL, get(AMDGPU::S_MOV_B32), DestReg)
             .addReg(SrcReg, getKillRegState(KillSrc));
     return;
 
   } else if (AMDGPU::SReg_64RegClass.contains(DestReg)) {
assert(AMDGPU::VReg_512RegClass.contains(SrcReg) ||
            AMDGPU::SReg_512RegClass.contains(SrcReg));
     Opcode = AMDGPU::V_MOV_B32_e32;
     SubIndices = Sub0_15;
 
   } else {
     llvm_unreachable("Can't copy register!");
   }
 
-  if (RI.getHWRegIndex(DestReg) <= RI.getHWRegIndex(SrcReg))
-    Forward = true;
-  else
-    Forward = false;
+  bool Forward = RI.getHWRegIndex(DestReg) <= RI.getHWRegIndex(SrcReg);
 
   for (unsigned Idx = 0; Idx < SubIndices.size(); ++Idx) {
     unsigned SubIdx;
     if (Forward)
       SubIdx = SubIndices[Idx];
     else
       SubIdx = SubIndices[SubIndices.size() - Idx - 1];
 
     MachineInstrBuilder Builder = BuildMI(MBB, MI, DL,
       get(Opcode), RI.getSubReg(DestReg, SubIdx));
 
     Builder.addReg(RI.getSubReg(SrcReg, SubIdx));
 
     if (Idx == SubIndices.size() - 1)
       Builder.addReg(SrcReg, getKillRegState(KillSrc) | RegState::Implicit);
 
     if (Idx == 0)
       Builder.addReg(DestReg, RegState::Define | RegState::Implicit);
+
+    Builder.addReg(SrcReg, RegState::Implicit);
   }
 }
 
 int SIInstrInfo::commuteOpcode(const MachineInstr &MI) const {
   const unsigned Opcode = MI.getOpcode();
 
   int NewOpc;
 

test/CodeGen/AMDGPU/undefined-subreg-liverange.ll

-; RUN: llc -verify-machineinstrs -o /dev/null %s
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck %s
 ; We may have subregister live ranges that are undefined on some paths. The
 ; verifier should not complain about this.
-target triple = "amdgcn--"
 
-define void @func() {
+
+; CHECK-LABEL: {{^}}func:
+define void @func() #0 {
 B0:
   br i1 undef, label %B1, label %B2
 
 B1:
   br label %B2
 
 B2:
   %v0 = phi <4 x float> [ zeroinitializer, %B1 ], [ <float 0.0, float 0.0, float 0.0, float undef>, %B0 ]
   br i1 undef, label %B30.1, label %B30.2
 
 B30.1:
   %sub = fsub <4 x float> %v0, undef
   br label %B30.2
 
 B30.2:
   %v3 = phi <4 x float> [ %sub, %B30.1 ], [ %v0, %B2 ]
   %ve0 = extractelement <4 x float> %v3, i32 0
   store float %ve0, float addrspace(3)* undef, align 4
   ret void
 }
+
+; FIXME: Extra undef subregister copy should be removed before
+; overwritten with defined copy
+; CHECK-LABEL: {{^}}valley_partially_undef_copy:
+define amdgpu_ps float @valley_partially_undef_copy() #0 {
+bb:
+  %tmp = load volatile i32, i32 addrspace(1)* undef, align 4
+  %tmp1 = load volatile i32, i32 addrspace(1)* undef, align 4
+  %tmp2 = insertelement <4 x i32> undef, i32 %tmp1, i32 0
+  %tmp3 = insertelement <4 x i32> %tmp2, i32 %tmp1, i32 1
+  %tmp4 = call <4 x float> @llvm.SI.image.sample.v4i32(<4 x i32> %tmp3, <8 x i32> undef, <4 x i32> undef, i32 15, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0)
+  %tmp5 = extractelement <4 x float> %tmp4, i32 0
+  %tmp6 = fmul float %tmp5, undef
+  %tmp7 = fadd float %tmp6, %tmp6
+  %tmp8 = insertelement <4 x i32> %tmp2, i32 %tmp, i32 1
+  store <4 x i32> %tmp8, <4 x i32> addrspace(1)* undef, align 16
+  store float %tmp7, float addrspace(1)* undef, align 4
+  br label %bb9
+
+bb9:                                              ; preds = %bb9, %bb
+  %tmp10 = icmp eq i32 %tmp, 0
+  br i1 %tmp10, label %bb9, label %bb11
+
+bb11:                                             ; preds = %bb9
+  store <4 x i32> %tmp2, <4 x i32> addrspace(1)* undef, align 16
+  ret float undef
+}
+
+; FIXME: Should be able to remove the undef copies
+
+; CHECK-LABEL: {{^}}partially_undef_copy:
+; CHECK: v_mov_b32_e32 v5, 5
+; CHECK: v_mov_b32_e32 v6, 6
+
+; CHECK: v_mov_b32_e32 v[[OUTPUT_LO:[0-9]+]], v5
+
+; Undef copy
+; CHECK: v_mov_b32_e32 v1, v6
+
+; undef copy
+; CHECK: v_mov_b32_e32 v2, v7
+
+; CHECK: v_mov_b32_e32 v[[OUTPUT_HI:[0-9]+]], v8
+; CHECK: v_mov_b32_e32 v[[OUTPUT_LO]], v6
+
+; CHECK: buffer_store_dwordx4 v{{\[}}[[OUTPUT_LO]]:[[OUTPUT_HI]]{{\]}}
+define void @partially_undef_copy() #0 {
+  %tmp0 = call i32 asm sideeffect "v_mov_b32_e32 v5, 5", "={VGPR5}"()
+  %tmp1 = call i32 asm sideeffect "v_mov_b32_e32 v6, 6", "={VGPR6}"()
+
+  %partially.undef.0 = insertelement <4 x i32> undef, i32 %tmp0, i32 0
+  %partially.undef.1 = insertelement <4 x i32> %partially.undef.0, i32 %tmp1, i32 0
+
+  store volatile <4 x i32> %partially.undef.1, <4 x i32> addrspace(1)* undef, align 16
+  tail call void asm sideeffect "v_nop", "v={VGPR5_VGPR6_VGPR7_VGPR8}"(<4 x i32> %partially.undef.0)
+  ret void
+}
+
+declare <4 x float> @llvm.SI.image.sample.v4i32(<4 x i32>, <8 x i32>, <4 x i32>, i32, i32, i32, i32, i32, i32, i32, i32) #1
+declare float @llvm.SI.image.sample.i32(i32, <8 x i32>, <4 x i32>, i32, i32, i32, i32, i32, i32, i32, i32) #1
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone }