<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-06-24 15:51 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"><div class="HOEnZb"><div class="h5"><br>
> On 2015 Jun 24, at 13:39, Alex Lorenz <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
><br>
> Hi dexonsmith, bob.wilson, bogner,<br>
><br>
> This patch is based on a previous serialization patch that serializes register mask machine operands (<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10673&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=rxkq7XknelBH5DoUcqsUuTWkdd4YerIkfFCfGkekh6g&s=lCMSNdV-o2HJkvoZPkDtgqYihVHhIfc9ExrM6oLONMM&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10673</a>).<br>
><br>
> This patch serializes the implicit register flag. This patch introduces two new keywords into the<br>
> machine instruction syntax: 'implicit' and 'implicit-def'. The 'implicit' keyword is used for the implicit<br>
> register operands, and the 'implicit-def' keyword is used for the register operands that have the<br>
> implicit and define flag. Examples:<br>
><br>
> CMP32ri8 %edi, 10, implicit-def %eflags<br>
> JG_1 %bb.2.exit, implicit %eflags<br>
><br>
> This patch also adds a way to control the implicit machine operands - if a parser encounters a<br>
> register operand with an 'implicit' or 'implicit-def' flag, it will assume that this instruction specifies<br>
> all the implicit operands explicitly. But if the parser encounters an instruction that has no implicit register<br>
> operands, it will add the implicit register operands that are specified by the instruction information.<br>
> This means that the instruction:<br>
><br>
> CMP32ri8 %edi, 10<br>
><br>
> Is equivalent to:<br>
><br>
> CMP32ri8 %edi, 10, implicit-def %eflags<br>
><br>
> REPOSITORY<br>
> rL LLVM<br>
><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10709&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=rxkq7XknelBH5DoUcqsUuTWkdd4YerIkfFCfGkekh6g&s=9x2ZKj-PWWgW-pcz4hwARkLxNh22qMbVVIdZVE_zSAU&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10709</a><br>
><br>
> Files:<br>
> lib/CodeGen/MIRParser/MILexer.cpp<br>
> lib/CodeGen/MIRParser/MILexer.h<br>
> lib/CodeGen/MIRParser/MIParser.cpp<br>
> lib/CodeGen/MIRPrinter.cpp<br>
> test/CodeGen/MIR/X86/expected-register-after-flags.mir<br>
> test/CodeGen/MIR/X86/implicit-register-flag.mir<br>
> test/CodeGen/MIR/X86/machine-instructions.mir<br>
> test/CodeGen/MIR/X86/register-mask-operands.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=rxkq7XknelBH5DoUcqsUuTWkdd4YerIkfFCfGkekh6g&s=1qb7iaXohF5iKSenZKlTg8ESNUin-lxZ9H0v1uC2Tvk&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</div></div>> <D10709.28394.patch><br>
> Index: lib/CodeGen/MIRParser/MILexer.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/MIRParser/MILexer.cpp<br>
> +++ lib/CodeGen/MIRParser/MILexer.cpp<br>
> @@ -169,22 +171,20 @@<br>
><br>
> const auto &MCID = MF.getSubtarget().getInstrInfo()->get(OpCode);<br>
><br>
> - // Verify machine operands.<br>
> + // Verify machine operands and determine if we should add the implicit<br>
> + // register machine operands.<br>
> + bool NoImplicit = false;<br>
> if (!MCID.isVariadic()) {<br>
> for (size_t I = 0, E = Operands.size(); I < E; ++I) {<br>
> + if (Operands[I].isReg() && Operands[I].isImplicit())<br>
> + NoImplicit = true;<br>
> if (I < MCID.getNumOperands())<br>
> continue;<br>
> - // Mark this register as implicit to prevent an assertion when it's added<br>
> - // to an instruction. This is a temporary workaround until the implicit<br>
> - // register flag can be parsed.<br>
> - if (Operands[I].isReg())<br>
> - Operands[I].setImplicit();<br>
> + // TODO: Check for extraneous machine operands.<br>
> }<br>
> }<br>
><br>
> - // TODO: Determine the implicit behaviour when implicit register flags are<br>
> - // parsed.<br>
> - auto *MI = MF.CreateMachineInstr(MCID, DebugLoc(), /*NoImplicit=*/true);<br>
> + auto *MI = MF.CreateMachineInstr(MCID, DebugLoc(), NoImplicit);<br>
<br>
This all seems a little magical to me. I'd rather, for now, be more<br>
conservative:<br>
<br>
- Continue to pass `/* NoImplicit */ true`.<br>
- Diagnostic (error? warning?) if the specified implicit operands<br>
don't match the default ones for the opcode. (Or does that get<br>
caught by the machine verifier?)<br>
<br>
Or maybe this is the kind of thing a .mir file could opt into via an<br>
`autoimplicit` keyword somewhere, such as:<br>
<br>
JG_1 %bb.2.exit, autoimplicit<br>
<br>
Thoughts from anyone else?<br></blockquote><div><br></div><div>Thanks,</div><div>I more conservative approach works for me. I will post an updated patch tomorrow.</div><div>We could also have a 'autoimplicit' property in the machine function YAML mapping that would apply 'autoimplicit' to every machine instruction automatically.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> for (const auto &Operand : Operands)<br>
> MI->addOperand(MF, Operand);<br>
> return MI;<br>
<br>
</blockquote></div><br></div></div>