[PATCH] D118233: [MachineVerifier] Report allocatable classes for physical register copies

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetRegisterInfo.cpp:229-242
 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;
----------------
arsenm wrote:
> 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
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`...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118233/new/

https://reviews.llvm.org/D118233