[AMDGPU] Handle subregisters properly in generic operand legalizer

[AditiRM]

Fix for the issue found during COPY introduction during legalization of PHI operands for sgpr to vgpr copy when subreg is involved.

The PHI node handling in "si-fix-sgpr-copies" is currently buggy. During the transformation, an illegal copy gets introduced (i.e., copy with input-output size mismatch) which in turn causes an illegal PHI. (see the below use case)

Input MIR	Output MIR
bb.0:	bb.0:
successors: %bb.1	successors: %bb.1
liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3	liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
%0:vreg_64 = V_MOV_B64_e32 0, implicit $exec	%0:vreg_64 = V_MOV_B64_e32 0, implicit $exec
%4:sreg_64 = COPY $sgpr0_sgpr1	%1:sreg_64 = COPY $sgpr0_sgpr1
%5:sreg_64 = COPY $sgpr2_sgpr3	%2:sreg_64 = COPY $sgpr2_sgpr3
bb.1:	bb.1:
successors: %bb.2	successors: %bb.2
%2:sreg_64 = S_ADD_U64_PSEUDO %4, %5, implicit-def $scc	%3:sreg_64 = S_ADD_U64_PSEUDO %1:sreg_64, %2:sreg_64, implicit-def $scc
S_BRANCH %bb.2	%7:vreg_64 = COPY %3.sub0:sreg_64, implicit $exec
	S_BRANCH %bb.2
bb.2:	bb.2:
successors: %bb.1, %bb.3	successors: %bb.1, %bb.3
%3:sreg_32 = PHI %1.sub0:sreg_64, %bb.3, %2.sub0:sreg_64, %bb.1	%6:vgpr_32 = PHI %0.sub0:vreg_64, %bb.3, %7:vreg_64, %bb.1
S_BRANCH %bb.3	S_BRANCH %bb.3
bb.3:	bb.3:
successors: %bb.2	successors: %bb.2
%1:sreg_64 = COPY %0	%5:sreg_64 = IMPLICIT_DEF
S_BRANCH %bb.2	S_BRANCH %bb.2

This change is relevant for : [AMDGPU] Add MachineVerifier check to detect illegal copies from vector register to SGPR

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Aditi Medhane (AditiRM)

Changes

Fix for the issue found during COPY introduction during legalization of PHI operands for sgpr to vgpr copy when subreg is involved

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+2-1)
• (modified) llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir (+2-2)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index e4a679f6a3ef8f..2102c608f8cb21 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6231,10 +6231,11 @@ void SIInstrInfo::legalizeGenericOperand(MachineBasicBlock &InsertMBB,
     return;
 
   Register DstReg = MRI.createVirtualRegister(DstRC);
+  Op.setSubReg(0);
   auto Copy = BuildMI(InsertMBB, I, DL, get(AMDGPU::COPY), DstReg).add(Op);
 
   Op.setReg(DstReg);
-  Op.setSubReg(0);
+  Op.setSubReg(OpSubReg);
 
   MachineInstr *Def = MRI.getVRegDef(OpReg);
   if (!Def)
diff --git a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
index dab4c9d401407b..d21dbd290accea 100644
--- a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
+++ b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
@@ -73,13 +73,13 @@ body:             |
   ; GCN-NEXT:   successors: %bb.2(0x80000000)
   ; GCN-NEXT: {{  $}}
   ; GCN-NEXT:   [[S_ADD_U:%[0-9]+]]:sreg_64 = S_ADD_U64_PSEUDO [[COPY]], [[COPY1]], implicit-def $scc
-  ; GCN-NEXT:   [[COPY2:%[0-9]+]]:vreg_64 = COPY [[S_ADD_U]].sub0, implicit $exec
+  ; GCN-NEXT:   [[COPY2:%[0-9]+]]:vreg_64 = COPY [[S_ADD_U]], implicit $exec
   ; GCN-NEXT:   S_BRANCH %bb.2
   ; GCN-NEXT: {{  $}}
   ; GCN-NEXT: bb.2:
   ; GCN-NEXT:   successors: %bb.3(0x80000000)
   ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[PHI:%[0-9]+]]:vgpr_32 = PHI [[V_MOV_B64_e32_]].sub0, %bb.3, [[COPY2]], %bb.1
+  ; GCN-NEXT:   [[PHI:%[0-9]+]]:vgpr_32 = PHI [[V_MOV_B64_e32_]].sub0, %bb.3, [[COPY2]].sub0, %bb.1
   ; GCN-NEXT:   S_BRANCH %bb.3
   ; GCN-NEXT: {{  $}}
   ; GCN-NEXT: bb.3:

[arsenm]

Instead of clearing this and using .add, could use .addReg

We probably don't want to be preserving the flags here. If this somehow ended up with a kill, it would be wrong

[arsenm]

Instead of clearing this and using .add, could use .addReg and just not take the subreg index

We probably don't want to be preserving the flags here. If this somehow ended up with a kill, it would be wrong

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.