[MachineVerifier] Report allocatable classes for physical register copies

[lewis-revill]

Summary

The method getMinimalPhysRegClassLLT is used by the machine verifier on gMIR to find the register class for a physical register operand of a copy to/from a physical register. It then uses the register class to determine the size of the register to verify that the copy has matching sizes.

If this method can report unallocatable register classes as the best match for a physical register then we can run into the issue where, since unallocatable register classes are used as dummy classes where the type/size is unknown (IE for RISC-V), then the reported size doesn't match that of the register class that the register actually belongs to.

This may not be the only/neatest way of solving this issue but the issue is a blocker for RISC-V GlobalISel support.

[arsenm]

llvm/lib/CodeGen/TargetRegisterInfo.cpp

229–242

This function was intended to follow along with the DAG version, which does not have the allocatable filter. It would probably be better to follow along with the pattern, and have the use have to explicitly filter with getAllocatableClass

[lewis-revill]

llvm/lib/CodeGen/TargetRegisterInfo.cpp

229–242

I can understand why it's best to match that. Problem is if this finds an unallocatable class first, even among multiple suitable allocatable classes then it will always report that. So the user can do nothing about it. I could try and rewrite it so that it reports the unallocatable class only if we don't find a suitable allocatable one. Another option is to add another property which matches this better than isAllocatable. Something like isDummyClass...

[arsenm]

llvm/lib/CodeGen/TargetRegisterInfo.cpp

229–242

I think it's important to keep the semantics of the two matching. What caller context are you having a problem? Code generally applies getAllocatableClass in conjunction with this

[lewis-revill]

llvm/lib/CodeGen/TargetRegisterInfo.cpp

229–242

I looked into this again, and the problem was that I was always getting this unallocatable class reported with pointer LLTs. However the reason is not actually because the unallocatable class is overriding the allocatable class encountered later, but actually because isTypeLegalForClass always returns false for pointer LLTs (since no MVT types map to LLT pointer types). So I will create a new patch which addresses this instead. This would fix my issue so I think this patch isn't relevant - the allocatable class would always override the unallocatable class anyway if the type was legal.

[lewis-revill]

Abandoned in favour of D118766

llvm/lib/CodeGen/TargetRegisterInfo.cpp

if ((VT == MVT::Other || isTypeLegalForClass(*RC, VT)) &&
       BestRC = RC;
   }
 
   assert(BestRC && "Couldn't find the register class");
   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)))
+    if ((!Ty.isValid() ||
+         (RC->isAllocatable() && 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,
                                    const TargetRegisterClass *RC, BitVector &R){
   assert(RC->isAllocatable() && "invalid for nonallocatable sets");