[PATCH] D37775: Add a verifier test to check the access on both sides of COPY are the same

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/CodeGen/MachineVerifier.cpp:977
+    const MachineOperand &DstOp = MI->getOperand(0);
+    const MachineOperand &SrcOp = MI->getOperand(1);
+    LLT DstTy = MRI->getType(DstOp.getReg());
----------------
aivchenk wrote:
> Here we rely on having operands in MI, while previously in MachineVerifier::visitMachineInstrBefore we could discover that there are not enough operands given.  Should we guard that with foundErrors?
> 
> test/CodeGen/MIR/X86/machine-verifier.mir
> fails due to that reason
Sounds sensible to me.


================
Comment at: lib/CodeGen/TargetRegisterInfo.cpp:429
+TargetRegisterInfo::getRegSizeInBits(unsigned Reg,
+                                     const MachineRegisterInfo &MRI) const {
+  const TargetRegisterClass *RC = nullptr;
----------------
This function is not consistent with RegisterBankInfo::getSizeInBits in the priority it gives to the different size when several sources are available (RC, Type).

RBI goes Type if any then RC, this one does the opposite and I believe this is not what we want.

At the very least, RBI should use this implementation instead of duplicating the logic.


https://reviews.llvm.org/D37775