[PATCH] D97054: [MachineVerifier] Confirm that both ends of a tied def/use are live together

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D97054#2601519 <https://reviews.llvm.org/D97054#2601519>, @reames wrote:

> In D97054#2580395 <https://reviews.llvm.org/D97054#2580395>, @arsenm wrote:
>
>> In D97054#2579436 <https://reviews.llvm.org/D97054#2579436>, @reames wrote:
>>
>>> In D97054#2574843 <https://reviews.llvm.org/D97054#2574843>, @arsenm wrote:
>>>
>>>> Testcase?
>>>
>>> Not sure how to honestly.  The whole idea is that an invalid live range should be impossible to construct, but that we want to catch it in verification if the register allocator does so.
>>
>> We have unit tests in unittests/MI/LiveIntervalTest
>
> I might be missing the obvious, but I don't see anything here to guide me on how to construct an intentionally malformed live interval and check for a verifier failure.

Well there's not really much to guide creating properly constructed intervals either...

Theoretically you could just do something like LIS->getInterval(Reg).removeSegment(Start, WrongEnd)



================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2749
+
+      if (tiedOpsRewritten(MF) && MOI->isTied() && Reg.isVirtual()) {
+        unsigned DefOpNum = std::distance(MI->operands_begin(), &*MOI);
----------------
Maybe should have a FIXME to handle physical registers too


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2754
+        // Both ends of a tied def must be the same interval.  Exceptions:
+        // - use is undef, implicit or noreg (not a real read)
+        // - live interval has subranges (purely because this is complicated
----------------
I don't see why being implicit would be a special case for this. Implicit operands still need to follow all of the liveness rules


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2757
+        //   and we haven't implemented the precise check yet)
+        if (!UseOp.isUndef() && !UseOp.isImplicit() && UseOp.getReg() != 0 &&
+            LaneMask.none() && !LR.liveAt(VNI->def.getBaseIndex())) {
----------------
I think checking isUndef is usually wrong, since undef on a subregister def still reads the register. This should probably be .readsReg()


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

https://reviews.llvm.org/D97054