MachineVerifier doesn't check live-in lists

[MatzeB]

Bugzilla Link	33182
Version	4.0
OS	All
CC	@RKSimon,@qcolombet

Extended Description

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.

[MatzeB]

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

[qcolombet]

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 :).

[MatzeB]

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.

[qcolombet]

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

[qcolombet]

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.

[MatzeB]

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.

[MatzeB]

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).

[qcolombet]

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

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

[MatzeB]

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.