[PATCH] D64650: Add a TargetMachineVerifier that runs along with the existing MachineVerifier

Fri Jul 12 15:27:27 PDT 2019

dsanders added a comment.

In D64650#1583590 <https://reviews.llvm.org/D64650#1583590>, @arsenm wrote:

> Having it as an independent pass may be kind of annoying. Having verifier runs in -print-before-all/-print-after-all is annoying enough already, and this would double them

The MachineVerifier itself is supposed to prevent it by calling PM->add() directly to add itself instead of going through addPass() but PM is private so I wasn't able to use the same method. Along the same lines the `false, false` in:

  addPass(createAArch64MachineVerifier(Banner), false, false);

is supposed to prevent that. One of them prevents verifiers being added after the pass while the other prevents printers being added after the pass. Interestingly, TargetMachineVerifier successfully elides the printer on an older branch but not on trunk, while MachineVerifier fails on both. I'll take a look.

In D64650#1583566 <https://reviews.llvm.org/D64650#1583566>, @arsenm wrote:

> In D64650#1583533 <https://reviews.llvm.org/D64650#1583533>, @dsanders wrote:
>
> > In D64650#1583208 <https://reviews.llvm.org/D64650#1583208>, @arsenm wrote:
> >
> > > The current target verification problems I have are:
> > >
> > > - Verifying legal register bank mappings, so I don't have to defend against them in InstructionSelect
> > > - Verifying G_INTRINSIC operand types and numbers. There's missing API to do this.
> >
>
>
> The only Intrinsic:: functions only deal with IR-level information. I didn't see a way to figure out the argument list without getting a concrete Function declaration. That would require converting from the argument LLTs to Type*, but you also don't know the context of which operands are needed for mangling without hardcoding it for specific intrinsics.

Ah ok. I haven't encountered a case where we need to reach back to the original argument list so I don't have any ideas for this to hand.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64650