MachineVerifier doesn't check live-in lists

[Quuxplusone]

Bugzilla Link	PR33182
Status	NEW
Importance	P enhancement
Reported by	Matthias Braun (matze@braunis.de)
Reported on	2017-05-26 10:55:34 -0700
Last modified on	2017-06-25 05:58:08 -0700
Version	4.0
Hardware	PC All
CC	jatin.bhateja@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, quentin.colombet@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also	PR32146

After some recent fixes I wondered why the machine verifier didn't catch the
problems. Turns out we do not check live-in lists:
Example:

> cat t.mir
# RUN: llc -o - %s -mtriple=aarch64-- -run-pass=none -verify-machineinstrs |
FileCheck %s
---
# CHECK-LABEL: name: func
name: func
tracksRegLiveness: true
body: |
  bb.0:
    liveins: %x5
    %x5 = ADDXrr killed %x5, killed %x5

  bb.1:
    liveins: %x10
    %x10 = ADDXrr killed %x10, killed %x10
...

The register %x10 claimed to be live-in in bb.1 is not live at the end of bb.0
yet the MachineVerifier doesn't complain.

[Quuxplusone]

Note that I do have some experimental patches that let the machine verifier check the liveins list but I cannot currently commit them as it uncovers bugs that need to be fixed first.

[Quuxplusone]

Are we planning to pull a full data flow algorithm here?

If yes, that sounds expensive to run between every pass.

[Quuxplusone]

Are we planning to pull a full data flow algorithm here?

No this is just verification not computation of the live-in lists. We just need to compare the live registers at the end of a block with the livein lists of the successors (with some exceptions like EHLanding pads having live values out of thin air, and our modeling of tailcalls being somewhat odd/strange).

[Quuxplusone]

And to add to that: Yes it's not minimal in the sense that the verifier would not complain if there are more registers in the live-in lists than necessary. But that is fine IMO, the important part is that we catch the cases where we have suddenly declare dead-registers as live because of wrong live-in lists.

[Quuxplusone]

(In reply to Matthias Braun from comment #3)
> live registers at the end of the block

How do you get that information?

Basically, I am trying to understand what kind of errors do we catch.

[Quuxplusone]

Put differently, if we do for instance `current live-ins + defs in block`, we
would catch the problem in your example, but not:
# CHECK-LABEL: name: func
name: func
tracksRegLiveness: true
body: |
  bb.0:
    liveins: %x5
    %x5 = ADDXrr killed %x5, killed %x5

  bb.1:
    liveins:
    %x10 = ADDXrr killed %x10, killed %x10 <-- missing live ins

[Quuxplusone]

We catch your last example already as we start simulating liveness from the start of the basic block and will complain at the kill without the register being live.

[Quuxplusone]

(In reply to Matthias Braun from comment #7)
> We catch your last example already as we start simulating liveness from the
> start of the basic block and will complain at the kill without the register
> being live.

Yeah, the point was could we construct (harmful) examples where whatever
solutions we put in place do not catch them?

My worry was that we would end up with an increasingly growing set of
heuristics to catch more and more cases instead of doing the right thing.

Given what you suggest to add and what we already check, I can't think of a
harmful example and thus, we don't seem to go in that direction, so I am happy
:).

[Quuxplusone]

See also https://reviews.llvm.org/D33650