[llvm] [AMDGPU] Add MachineVerifer check to detect illegal copies from VGPR to SGPR (PR #105494)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 21 03:09:51 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Aditi Medhane (AditiRM)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/105494.diff
6 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+29-3)
- (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.h (+3)
- (added) llvm/test/CodeGen/AMDGPU/fix-illegal-vgpr-copies.mir (+29)
- (added) llvm/test/CodeGen/AMDGPU/phi-moveimm-subreg-input.mir (+30)
- (modified) llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir (-32)
- (modified) llvm/test/CodeGen/AMDGPU/wqm.mir (+4-5)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index b6dd4905fb61bb..22572a92227b70 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4613,15 +4613,41 @@ static bool isSubRegOf(const SIRegisterInfo &TRI,
SubReg.getReg() == SuperVec.getReg();
}
+// Verify the illgal copy from VGPR to SGPR for generic opcode COPY
+bool SIInstrInfo::verifyCopy(const MachineInstr &MI,
+ const MachineRegisterInfo &MRI,
+ StringRef &ErrInfo) const {
+ const MachineOperand &Dst = MI.getOperand(0);
+ const MachineOperand &Src = MI.getOperand(1);
+
+ if (Dst.isReg() && Src.isReg()) {
+ Register DstReg = Dst.getReg();
+ Register SrcReg = Src.getReg();
+ // This is a check for copy from an VGPR to SGPR
+ if (RI.isVGPR(MRI, SrcReg) && RI.isSGPRReg(MRI, DstReg)) {
+ ErrInfo = "illegal copy from VGPR 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();
+ if (SIInstrInfo::isGenericOpcode(MI.getOpcode())) {
+ // 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.
+ if (!MRI.isSSA() && MI.isCopy())
+ return verifyCopy(MI, MRI, ErrInfo);
+
+ 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 1712dfe8d406cc..4caf37cd2f08e0 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/fix-illegal-vgpr-copies.mir b/llvm/test/CodeGen/AMDGPU/fix-illegal-vgpr-copies.mir
new file mode 100644
index 00000000000000..8eaab0a16e55e4
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/fix-illegal-vgpr-copies.mir
@@ -0,0 +1,29 @@
+# RUN: not --crash llc -march=amdgcn -mcpu=gfx1200 -run-pass=machineverifier %s 2>&1 | FileCheck -check-prefix=ERR %s
+
+---
+name: fix-illegal-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
+
+ ; ERR: *** Bad machine code: illegal copy from VGPR to SGPR ***
+ ; ERR: instruction: %4:sgpr_32 = COPY %0:vgpr_32
+ %4:sgpr_32 = COPY %0:vgpr_32
+
+ ; ERR: *** Bad machine code: illegal copy from VGPR to SGPR ***
+ ; ERR: instruction: $sgpr0 = COPY %0:vgpr_32
+ $sgpr0 = COPY %0:vgpr_32
+
+ ; ERR: *** Bad machine code: illegal copy from VGPR to SGPR ***
+ ; ERR: instruction: $sgpr1 = COPY $vgpr0
+ $sgpr1 = COPY $vgpr0
+
+ S_ENDPGM 0
+...
diff --git a/llvm/test/CodeGen/AMDGPU/phi-moveimm-subreg-input.mir b/llvm/test/CodeGen/AMDGPU/phi-moveimm-subreg-input.mir
new file mode 100644
index 00000000000000..5d58673f7bdca9
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/phi-moveimm-subreg-input.mir
@@ -0,0 +1,30 @@
+# RUN: not --crash llc -mtriple=amdgcn -mcpu=gfx900 -verify-machineinstrs -run-pass=si-fix-sgpr-copies -o - %s 2>&1 | FileCheck -check-prefix=GCN %s
+
+# GCN: *** Bad machine code: illegal copy from VGPR to SGPR ***
+# GCN: instruction: undef %5.sub0:sreg_64 = COPY %0:vgpr_32
+name: phi_moveimm_subreg_input
+tracksRegLiveness: true
+body: |
+ bb.0:
+ successors: %bb.1
+ liveins: $sgpr0, $sgpr1
+
+ %0:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
+
+ %4:sreg_32 = COPY $sgpr0
+ %5:sreg_32 = COPY $sgpr1
+
+ bb.1:
+ successors: %bb.2
+ undef %2.sub0:sreg_64 = S_ADD_U32 %4, %5, implicit-def $scc
+ S_BRANCH %bb.2
+
+ bb.2:
+ successors: %bb.3
+ %3:sreg_64 = PHI %1, %bb.3, %2, %bb.1
+ S_BRANCH %bb.3
+
+ bb.3:
+ successors: %bb.2
+ undef %1.sub0:sreg_64 = COPY %0
+ S_BRANCH %bb.2
diff --git a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
index f931acb8408da2..2b78f984ae775c 100644
--- a/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
+++ b/llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
@@ -32,38 +32,6 @@ body: |
S_BRANCH %bb.2
...
----
-# GCN-LABEL: name: phi_moveimm_subreg_input
-# GCN: %{{[0-9]+}}:sreg_64 = PHI %{{[0-9]+}}, %bb.3, %{{[0-9]+}}, %bb.1
-name: phi_moveimm_subreg_input
-tracksRegLiveness: true
-body: |
- bb.0:
- successors: %bb.1
- liveins: $sgpr0, $sgpr1
-
- %0:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
-
- %4:sreg_32 = COPY $sgpr0
- %5:sreg_32 = COPY $sgpr1
-
- bb.1:
- successors: %bb.2
- undef %2.sub0:sreg_64 = S_ADD_U32 %4, %5, implicit-def $scc
- S_BRANCH %bb.2
-
- bb.2:
- successors: %bb.3
- %3:sreg_64 = PHI %1, %bb.3, %2, %bb.1
- S_BRANCH %bb.3
-
- bb.3:
- successors: %bb.2
- undef %1.sub0:sreg_64 = COPY %0
- S_BRANCH %bb.2
-...
-
-
---
# GCN-LABEL: name: phi_moveimm_bad_opcode_input
# GCN-NOT: %{{[0-9]+}}:sreg_32 = PHI %{{[0-9]+}}, %bb.3, %{{[0-9]+}}, %bb.1
diff --git a/llvm/test/CodeGen/AMDGPU/wqm.mir b/llvm/test/CodeGen/AMDGPU/wqm.mir
index ef6d0780f395fd..5ff508b1e3842e 100644
--- a/llvm/test/CodeGen/AMDGPU/wqm.mir
+++ b/llvm/test/CodeGen/AMDGPU/wqm.mir
@@ -189,9 +189,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
@@ -212,10 +212,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
...
``````````
</details>
https://github.com/llvm/llvm-project/pull/105494
More information about the llvm-commits
mailing list