[llvm] cea97fc - GlobalISel: Relax verification of physical register copy types
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 28 05:45:47 PDT 2021
Author: Matt Arsenault
Date: 2021-04-28T08:45:41-04:00
New Revision: cea97fc0fcd86d52f9efa215116356b72faeb17d
URL: https://github.com/llvm/llvm-project/commit/cea97fc0fcd86d52f9efa215116356b72faeb17d
DIFF: https://github.com/llvm/llvm-project/commit/cea97fc0fcd86d52f9efa215116356b72faeb17d.diff
LOG: GlobalISel: Relax verification of physical register copy types
This was picking a concrete size for a physical register, and
enforcing exact match on the virtual register's type size. Some
targets add multiple types to a register class, and some are smaller
than the full bit width. For example x86 adds f32 to 128-bit xmm
registers, and AMDGPU adds i16/f16 to 32-bit registers.
It might be better to represent these cases as a copy of the full
register and an extraction of the subpart, but a lot of code assumes
you can directly copy. This will help fix the current usage of the DAG
calling convention infrastructure which is incompatible with how
GlobalISel is now using it.
The API is somewhat cumbersome here, but I just mirrored the existing
functions, except now with LLTs (and allow returning null on failure,
unlike the MVT version). I think the concept of selecting register
classes based on type is flawed to begin with, but I'm trying to keep
this compatible with the existing handling.
Added:
llvm/test/MachineVerifier/test_copy_physregs_x86.mir
Modified:
llvm/include/llvm/CodeGen/TargetRegisterInfo.h
llvm/lib/CodeGen/MachineVerifier.cpp
llvm/lib/CodeGen/TargetRegisterInfo.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 9c34a0e7b6aa5..954d043d7a7be 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -301,6 +301,19 @@ class TargetRegisterInfo : public MCRegisterInfo {
return false;
}
+ /// Return true if the given TargetRegisterClass is compatible with LLT T.
+ bool isTypeLegalForClass(const TargetRegisterClass &RC, LLT T) const {
+ for (auto I = legalclasstypes_begin(RC); *I != MVT::Other; ++I) {
+ MVT VT(*I);
+ if (VT == MVT::Untyped)
+ return true;
+
+ if (LLT(VT) == T)
+ return true;
+ }
+ return false;
+ }
+
/// Loop over all of the value types that can be represented by values
/// in the given register class.
vt_iterator legalclasstypes_begin(const TargetRegisterClass &RC) const {
@@ -320,6 +333,13 @@ class TargetRegisterInfo : public MCRegisterInfo {
const TargetRegisterClass *getMinimalPhysRegClass(MCRegister Reg,
MVT VT = MVT::Other) const;
+ /// Returns the Register Class of a physical register of the given type,
+ /// picking the most sub register class of the right type that contains this
+ /// physreg. If there is no register class compatible with the given type,
+ /// returns nullptr.
+ const TargetRegisterClass *getMinimalPhysRegClassLLT(MCRegister Reg,
+ LLT Ty = LLT()) const;
+
/// Return the maximal subclass of the given register class that is
/// allocatable or NULL.
const TargetRegisterClass *
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index e62b2195d915d..cb098acb960b5 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1678,28 +1678,54 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
case TargetOpcode::COPY: {
const MachineOperand &DstOp = MI->getOperand(0);
const MachineOperand &SrcOp = MI->getOperand(1);
- LLT DstTy = MRI->getType(DstOp.getReg());
- LLT SrcTy = MRI->getType(SrcOp.getReg());
+ const Register SrcReg = SrcOp.getReg();
+ const Register DstReg = DstOp.getReg();
+
+ LLT DstTy = MRI->getType(DstReg);
+ LLT SrcTy = MRI->getType(SrcReg);
if (SrcTy.isValid() && DstTy.isValid()) {
// If both types are valid, check that the types are the same.
if (SrcTy != DstTy) {
report("Copy Instruction is illegal with mismatching types", MI);
errs() << "Def = " << DstTy << ", Src = " << SrcTy << "\n";
}
+
+ break;
}
- if (SrcTy.isValid() || DstTy.isValid()) {
- // If one of them have valid types, let's just check they have the same
- // size.
- unsigned SrcSize = TRI->getRegSizeInBits(SrcOp.getReg(), *MRI);
- unsigned DstSize = TRI->getRegSizeInBits(DstOp.getReg(), *MRI);
- assert(SrcSize && "Expecting size here");
- assert(DstSize && "Expecting size here");
- if (SrcSize != DstSize)
- if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
- report("Copy Instruction is illegal with mismatching sizes", MI);
- errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
- << "\n";
- }
+
+ if (!SrcTy.isValid() && !DstTy.isValid())
+ break;
+
+ // If we have only one valid type, this is likely a copy between a virtual
+ // and physical register.
+ unsigned SrcSize = 0;
+ unsigned DstSize = 0;
+ if (SrcReg.isPhysical() && DstTy.isValid()) {
+ const TargetRegisterClass *SrcRC =
+ TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
+ if (SrcRC)
+ SrcSize = TRI->getRegSizeInBits(*SrcRC);
+ }
+
+ if (SrcSize == 0)
+ SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
+
+ if (DstReg.isPhysical() && SrcTy.isValid()) {
+ const TargetRegisterClass *DstRC =
+ TRI->getMinimalPhysRegClassLLT(DstReg, SrcTy);
+ if (DstRC)
+ DstSize = TRI->getRegSizeInBits(*DstRC);
+ }
+
+ if (DstSize == 0)
+ DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
+
+ if (SrcSize != 0 && DstSize != 0 && SrcSize != DstSize) {
+ if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
+ report("Copy Instruction is illegal with mismatching sizes", MI);
+ errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
+ << "\n";
+ }
}
break;
}
diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
index 22e1d5408740a..e95e089e63ae8 100644
--- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
+++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
@@ -225,6 +225,23 @@ TargetRegisterInfo::getMinimalPhysRegClass(MCRegister reg, MVT VT) const {
return BestRC;
}
+const TargetRegisterClass *
+TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
+ assert(Register::isPhysicalRegister(reg) &&
+ "reg must be a physical register");
+
+ // Pick the most sub register class of the right type that contains
+ // this physreg.
+ const TargetRegisterClass *BestRC = nullptr;
+ for (const TargetRegisterClass *RC : regclasses()) {
+ if ((!Ty.isValid() || isTypeLegalForClass(*RC, Ty)) && RC->contains(reg) &&
+ (!BestRC || BestRC->hasSubClass(RC)))
+ BestRC = RC;
+ }
+
+ return BestRC;
+}
+
/// getAllocatableSetForRC - Toggle the bits that represent allocatable
/// registers for the specific register class.
static void getAllocatableSetForRC(const MachineFunction &MF,
diff --git a/llvm/test/MachineVerifier/test_copy_physregs_x86.mir b/llvm/test/MachineVerifier/test_copy_physregs_x86.mir
new file mode 100644
index 0000000000000..7a91aaedb8679
--- /dev/null
+++ b/llvm/test/MachineVerifier/test_copy_physregs_x86.mir
@@ -0,0 +1,54 @@
+#RUN: not --crash llc -march=x86-64 -run-pass=none -verify-machineinstrs -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: x86-registered-target
+
+# These copies have mismatched type sizes that are allowed because the
+# register class is defined to directly include the narrower type.
+---
+name: test_valid_copies
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $xmm0, $xmm1, $xmm2, $xmm3, $xmm4
+ %0:_(s32) = COPY $xmm0
+ %1:_(s64) = COPY $xmm1
+ %2:_(s128) = COPY $xmm2
+ %3:_(<4 x s32>) = COPY $xmm3
+ %4:_(<2 x s64>) = COPY $xmm4
+ $xmm0 = COPY %0
+ $xmm1 = COPY %1
+ $xmm2 = COPY %2
+ $xmm3 = COPY %3
+ $xmm4 = COPY %4
+...
+
+---
+name: test_invalid_copies
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $xmm0, $xmm1, $xmm2, $xmm3
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ %0:_(s16) = COPY $xmm0
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ %1:_(<4 x s16>) = COPY $xmm1
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ %2:_(s256) = COPY $xmm2
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ %3:_(<8 x s32>) = COPY $xmm3
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ $xmm0 = COPY %0
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ $xmm1 = COPY %1
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ $xmm2 = COPY %2
+
+ ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+ $xmm3 = COPY %3
+...
More information about the llvm-commits
mailing list