[PATCH] MIR Parser: verify the implicit register operands.

[Alex L]

I agree that it would make more sense for these checks to be in the
verifier, but one advantage of doing the checks in the parser is that the
parser has access to the source locations and can report better diagnostics
because of that.


2015-06-27 8:27 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015 Jun 26, at 15:34, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bob.wilson, bogner,
> >
>
> +quentin
>
> > This patch is based on a previous serialization patch that serializes
> implicit register flags (http://reviews.llvm.org/D10709).
> >
> > This patch verifies that the parsed machine instructions have implicit
> register operands as specified by MCInstrDesc. Variadic and call
> instructions aren't verified.
> >
>
> I'm not sure this is the right long term direction.  This seems better:
>
>  1. Implicitly add the `-machineverifier` pass to the beginning of the
>     pass pipeline.  Alternatively, create a free function that calls the
>     machine verifier right after parsing MIR (much like `verifyModule()`
>     at the IR-level).
>  2. Assuming these checks are valid (are instructions *incorrect* if they
>     list the wrong implicit flags) and missing from the machine verifier,
>     add them there.
>
> Quentin, what do you think?
>
> Regardless of the long term, this obviously is useful in its current form
> for bootstrapping MIR serialization.  We can move/delete/change it later.
> I say commit it with a well-placed:
>
>     // FIXME: Move to the -machineverifier.
>
> LGTM with that.
>
> BTW, most of the review I'm doing here could just as easily be done
> post-commit, especially given that you basically own MIR serialization ;).
>
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10781
> >
> > Files:
> >  lib/CodeGen/MIRParser/MIParser.cpp
> >  test/CodeGen/MIR/X86/expected-different-implicit-operand.mir
> >  test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir
> >  test/CodeGen/MIR/X86/expected-number-after-bb.mir
> >  test/CodeGen/MIR/X86/global-value-operands.mir
> >  test/CodeGen/MIR/X86/large-index-number-error.mir
> >  test/CodeGen/MIR/X86/machine-basic-block-operands.mir
> >  test/CodeGen/MIR/X86/machine-instructions.mir
> >  test/CodeGen/MIR/X86/missing-implicit-operand.mir
> >  test/CodeGen/MIR/X86/named-registers.mir
> >  test/CodeGen/MIR/X86/register-mask-operands.mir
> >  test/CodeGen/MIR/X86/unknown-machine-basic-block.mir
> >  test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D10781.28604.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150627/bbb33432/attachment.html>