[llvm] 60a8b2b - [AMDGPU] Add MachineVerifier check to detect illegal copies from vector register to SGPR (#105494)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 19 01:27:48 PDT 2024
Author: Aditi Medhane
Date: 2024-09-19T13:57:44+05:30
New Revision: 60a8b2b1d0842e257e2add6fb1b27cf45699b641
URL: https://github.com/llvm/llvm-project/commit/60a8b2b1d0842e257e2add6fb1b27cf45699b641
DIFF: https://github.com/llvm/llvm-project/commit/60a8b2b1d0842e257e2add6fb1b27cf45699b641.diff
LOG: [AMDGPU] Add MachineVerifier check to detect illegal copies from vector register to SGPR (#105494)
Addition of a check in the MachineVerifier to detect and report illegal
vector registers to SGPR copies in the AMDGPU backend, ensuring correct
code generation.
We can enforce this check only after SIFixSGPRCopies pass.
This is half-fix in the pipeline with the help of isSSA MachineFuction
property, the check is happening for passes after phi-node-elimination.
Added:
llvm/test/MachineVerifier/AMDGPU/fix-illegal-vector-copies.mir
Modified:
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
llvm/lib/Target/AMDGPU/SIInstrInfo.h
llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
llvm/test/CodeGen/AMDGPU/wqm.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 30aa36be99c95f..c6a9a627d457e7 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4600,15 +4600,38 @@ static bool isSubRegOf(const SIRegisterInfo &TRI,
SubReg.getReg() == SuperVec.getReg();
}
+// Verify the illegal copy from vector register to SGPR for generic opcode COPY
+bool SIInstrInfo::verifyCopy(const MachineInstr &MI,
+ const MachineRegisterInfo &MRI,
+ StringRef &ErrInfo) const {
+ Register DstReg = MI.getOperand(0).getReg();
+ Register SrcReg = MI.getOperand(1).getReg();
+ // This is a check for copy from vector register to SGPR
+ if (RI.isVectorRegister(MRI, SrcReg) && RI.isSGPRReg(MRI, DstReg)) {
+ ErrInfo = "illegal copy from vector register to SGPR";
+ return false;
+ }
+ return true;
+}
+
bool SIInstrInfo::verifyInstruction(const MachineInstr &MI,
StringRef &ErrInfo) const {
uint16_t Opcode = MI.getOpcode();
- if (SIInstrInfo::isGenericOpcode(MI.getOpcode()))
- return true;
-
const MachineFunction *MF = MI.getParent()->getParent();
const MachineRegisterInfo &MRI = MF->getRegInfo();
+ // FIXME: At this point the COPY verify is done only for non-ssa forms.
+ // Find a better property to recognize the point where instruction selection
+ // is just done.
+ // We can only enforce this check after SIFixSGPRCopies pass so that the
+ // illegal copies are legalized and thereafter we don't expect a pass
+ // inserting similar copies.
+ if (!MRI.isSSA() && MI.isCopy())
+ return verifyCopy(MI, MRI, ErrInfo);
+
+ if (SIInstrInfo::isGenericOpcode(MI.getOpcode()))
+ return true;
+
int Src0Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0);
int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
int Src2Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2);
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 4fd9b4366159be..d560792aa1a894 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -178,6 +178,9 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
Register findUsedSGPR(const MachineInstr &MI, int OpIndices[3]) const;
+ bool verifyCopy(const MachineInstr &MI, const MachineRegisterInfo &MRI,
+ StringRef &ErrInfo) const;
+
protected:
/// If the specific machine instruction is a instruction that moves/copies
/// value from one register to another register return destination and source
diff --git a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
index d21dbd290accea..c2c5340639a16b 100644
--- a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
+++ b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
@@ -112,7 +112,6 @@ body: |
S_BRANCH %bb.2
...
-
---
name: phi_moveimm_bad_opcode_input
tracksRegLiveness: true
diff --git a/llvm/test/CodeGen/AMDGPU/wqm.mir b/llvm/test/CodeGen/AMDGPU/wqm.mir
index 3013aabbd3bd42..4762760c4ba24b 100644
--- a/llvm/test/CodeGen/AMDGPU/wqm.mir
+++ b/llvm/test/CodeGen/AMDGPU/wqm.mir
@@ -190,9 +190,9 @@ body: |
# Ensure that strict_wwm is not put around an EXEC copy
#CHECK-LABEL: name: copy_exec
#CHECK: %7:sreg_64 = COPY $exec
-#CHECK-NEXT: %14:sreg_64 = ENTER_STRICT_WWM -1, implicit-def $exec, implicit-def $scc, implicit $exec
+#CHECK-NEXT: %13:sreg_64 = ENTER_STRICT_WWM -1, implicit-def $exec, implicit-def $scc, implicit $exec
#CHECK-NEXT: %8:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
-#CHECK-NEXT: $exec = EXIT_STRICT_WWM %14
+#CHECK-NEXT: $exec = EXIT_STRICT_WWM %13
#CHECK-NEXT: %9:vgpr_32 = V_MBCNT_LO_U32_B32_e64 %7.sub0, 0, implicit $exec
name: copy_exec
tracksRegLiveness: true
@@ -213,10 +213,9 @@ body: |
%10:vgpr_32 = V_MBCNT_LO_U32_B32_e64 %8.sub0:sreg_64, 0, implicit $exec
%11:vgpr_32 = V_MOV_B32_dpp %9:vgpr_32, %10:vgpr_32, 312, 15, 15, 0, implicit $exec
%12:sreg_32 = V_READLANE_B32 %11:vgpr_32, 63
- early-clobber %13:sreg_32 = STRICT_WWM %9:vgpr_32, implicit $exec
+ early-clobber %13:vgpr_32 = STRICT_WWM %9:vgpr_32, implicit $exec
- %14:vgpr_32 = COPY %13
- BUFFER_STORE_DWORD_OFFSET_exact killed %14, %4, %5, 4, 0, 0, implicit $exec
+ BUFFER_STORE_DWORD_OFFSET_exact killed %13, %4, %5, 4, 0, 0, implicit $exec
S_ENDPGM 0
...
diff --git a/llvm/test/MachineVerifier/AMDGPU/fix-illegal-vector-copies.mir b/llvm/test/MachineVerifier/AMDGPU/fix-illegal-vector-copies.mir
new file mode 100644
index 00000000000000..edafd3825374f7
--- /dev/null
+++ b/llvm/test/MachineVerifier/AMDGPU/fix-illegal-vector-copies.mir
@@ -0,0 +1,59 @@
+# RUN: not --crash llc -march=amdgcn -mcpu=gfx1200 -run-pass=none -o /dev/null %s 2>&1 | FileCheck %s
+
+---
+name: fix-illegal-vector-copies
+tracksRegLiveness: true
+machineFunctionInfo:
+ isEntryFunction: true
+body: |
+ bb.0:
+ %0:vgpr_32 = IMPLICIT_DEF
+ %0:vgpr_32 = IMPLICIT_DEF ; Break SSA format
+ %1:vgpr_32 = IMPLICIT_DEF
+ %2:sgpr_32 = IMPLICIT_DEF
+ %3:sgpr_32 = IMPLICIT_DEF
+ %4:agpr_32 = IMPLICIT_DEF
+ %5:agpr_32 = IMPLICIT_DEF
+
+ ; copy from virtual VGPR to virtual SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: %6:sgpr_32 = COPY %0:vgpr_32
+ %6:sgpr_32 = COPY %0:vgpr_32
+
+ ; copy from virtual VGPR to physical SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr0 = COPY %0:vgpr_32
+ $sgpr0 = COPY %0:vgpr_32
+
+ ; copy from physical VGPR to physical SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr1 = COPY $vgpr0
+ $sgpr1 = COPY $vgpr0
+
+ ; copy from virtual AGPR to virtual SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: %7:sgpr_32 = COPY %4:agpr_32
+ %7:sgpr_32 = COPY %4:agpr_32
+
+ ; copy from virtual AGPR to physical SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr2 = COPY %4:agpr_32
+ $sgpr2 = COPY %4:agpr_32
+
+ ; copy from physical AGPR to physical SGPR
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr3 = COPY $agpr0
+ $sgpr3 = COPY $agpr0
+
+ ; copy from tuple of physical VGPRs to tuple of physical SGPRs
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr4_sgpr5 = COPY $vgpr0_vgpr1
+ $sgpr4_sgpr5 = COPY $vgpr0_vgpr1
+
+ ; copy from tuple of physical AGPRs to tuple of physical SGPRs
+ ; CHECK: *** Bad machine code: illegal copy from vector register to SGPR ***
+ ; CHECK: - instruction: $sgpr6_sgpr7 = COPY $agpr0_agpr1
+ $sgpr6_sgpr7 = COPY $agpr0_agpr1
+
+ S_ENDPGM 0
+...
More information about the llvm-commits
mailing list