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

[Alex L]

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.
>
>
Thanks, I will commit this patch with the comment in the code.


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

I feel like a pre-commit review would be better for some of the MIR
patches, especially
when it comes to patches that heavily modify the syntax of the format. I
will commit
some of the obvious ones without pre-commit reviews though :)

Alex


>
> > 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/20150706/2fd07676/attachment.html>