<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-06-27 8:27 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On 2015 Jun 26, at 15:34, Alex Lorenz <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
><br>
> Hi dexonsmith, bob.wilson, bogner,<br>
><br>
<br>
+quentin<br>
<span class=""><br>
> This patch is based on a previous serialization patch that serializes implicit register flags (<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10709&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=l3qbpy6H2iA8XWG7kUFOwlEsi3vQ_9-9bGKgiWZRccc&s=0YdieO3R_lr3uDpArhsLrNBe5kvmxjABizwa0k6yrKI&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10709</a>).<br>
><br>
> This patch verifies that the parsed machine instructions have implicit register operands as specified by MCInstrDesc. Variadic and call instructions aren't verified.<br>
><br>
<br>
</span>I'm not sure this is the right long term direction.  This seems better:<br>
<br>
 1. Implicitly add the `-machineverifier` pass to the beginning of the<br>
    pass pipeline.  Alternatively, create a free function that calls the<br>
    machine verifier right after parsing MIR (much like `verifyModule()`<br>
    at the IR-level).<br>
 2. Assuming these checks are valid (are instructions *incorrect* if they<br>
    list the wrong implicit flags) and missing from the machine verifier,<br>
    add them there.<br>
<br>
Quentin, what do you think?<br>
<br>
Regardless of the long term, this obviously is useful in its current form<br>
for bootstrapping MIR serialization.  We can move/delete/change it later.<br>
I say commit it with a well-placed:<br>
<br>
    // FIXME: Move to the -machineverifier.<br>
<br>
LGTM with that.<br>
<br></blockquote><div> </div><div>Thanks, I will commit this patch with the comment in the code.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
BTW, most of the review I'm doing here could just as easily be done<br>
post-commit, especially given that you basically own MIR serialization ;).<br></blockquote><div><br></div><div>I feel <span style="line-height:normal">like a pre-commit review would be better for some of the MIR patches, especially</span></div><div><span style="line-height:normal">when </span><span style="line-height:normal">it comes to patches that heavily modify the syntax of the format. I will </span><span style="line-height:normal">commit</span></div><div><span style="line-height:normal">some of the obvious ones without pre-commit reviews though :)</span></div><div><br></div><div>Alex</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> REPOSITORY<br>
>  rL LLVM<br>
><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10781&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=l3qbpy6H2iA8XWG7kUFOwlEsi3vQ_9-9bGKgiWZRccc&s=U0bJFKO8H8BB1AYK5FKdSuFpuPZlC5Fz5AuQZ4A8gew&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10781</a><br>
><br>
> Files:<br>
>  lib/CodeGen/MIRParser/MIParser.cpp<br>
>  test/CodeGen/MIR/X86/expected-different-implicit-operand.mir<br>
>  test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir<br>
>  test/CodeGen/MIR/X86/expected-number-after-bb.mir<br>
>  test/CodeGen/MIR/X86/global-value-operands.mir<br>
>  test/CodeGen/MIR/X86/large-index-number-error.mir<br>
>  test/CodeGen/MIR/X86/machine-basic-block-operands.mir<br>
>  test/CodeGen/MIR/X86/machine-instructions.mir<br>
>  test/CodeGen/MIR/X86/missing-implicit-operand.mir<br>
>  test/CodeGen/MIR/X86/named-registers.mir<br>
>  test/CodeGen/MIR/X86/register-mask-operands.mir<br>
>  test/CodeGen/MIR/X86/unknown-machine-basic-block.mir<br>
>  test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir<br>
><br>
> EMAIL PREFERENCES<br>
>  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=l3qbpy6H2iA8XWG7kUFOwlEsi3vQ_9-9bGKgiWZRccc&s=J024qbKJDob911IuVUVbuVS71WhoQsujSzM2qvXT6oo&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</span>> <D10781.28604.patch><br>
<br>
</blockquote></div><br></div></div>