[PATCH] D34394: [MachineVerifier] Add check that tied physregs aren't different.

Jesper Antonsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 21 00:59:08 PDT 2017


JesperAntonsson added inline comments.


================
Comment at: lib/CodeGen/MachineVerifier.cpp:993
+        else if (!TargetRegisterInfo::isPhysicalRegister(MOTied.getReg()))
+          report("Tied counterpart must also be physical", &MOTied, TiedTo);
+        else if (MO->getReg() != MOTied.getReg())
----------------
qcolombet wrote:
> I'm not sure about that one.
> Theoretically, you could have a not yet allocated vreg here. As long as the allocation does the right thing with the tie, that isn't an error.
Thanks for reviewing. Ok, so it's allowed to mix physregs and vregs? I'll upload a new patch that removes this check then.


================
Comment at: test/CodeGen/MIR/X86/subregister-index-operands.mir:27
+    %0 = EXTRACT_SUBREG %eax, %subreg.sub_8bit_hi
+    %ax = REG_SEQUENCE %0, %subreg.sub_8bit, %0, %subreg.sub_8bit_hi
     RETQ %ax
----------------
qcolombet wrote:
> I would expect we add a new test instead of modifying this one.
> In particular I don't think this one is going to fail when you modify the isPhysicalRegister check that I mentioned earlier. 
Agreed, it won't fail, so I'll just leave this test as is.


https://reviews.llvm.org/D34394





More information about the llvm-commits mailing list