[PATCH] MIR Serialization: Serialize the implicit register flags.

Alex L arphaman at gmail.com
Thu Jun 25 13:23:37 PDT 2015


2015-06-24 15:51 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015 Jun 24, at 13:39, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bob.wilson, bogner,
> >
> > This patch is based on a previous serialization patch that serializes
> register mask machine operands (http://reviews.llvm.org/D10673).
> >
> > This patch serializes the implicit register flag. This patch introduces
> two new keywords into the
> > machine instruction syntax: 'implicit' and 'implicit-def'. The
> 'implicit' keyword is used for the implicit
> > register operands, and the 'implicit-def' keyword is used for the
> register operands that have the
> > implicit and define flag. Examples:
> >
> >      CMP32ri8 %edi, 10, implicit-def %eflags
> >      JG_1 %bb.2.exit, implicit %eflags
> >
> > This patch also adds a way to control the implicit machine operands - if
> a parser encounters a
> > register operand with an 'implicit' or 'implicit-def' flag, it will
> assume that this instruction specifies
> > all the implicit operands explicitly. But if the parser encounters an
> instruction that has no implicit register
> > operands, it will add the implicit register operands that are specified
> by the instruction information.
> > This means that the instruction:
> >
> >      CMP32ri8 %edi, 10
> >
> > Is equivalent to:
> >
> >      CMP32ri8 %edi, 10, implicit-def %eflags
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10709
> >
> > Files:
> >  lib/CodeGen/MIRParser/MILexer.cpp
> >  lib/CodeGen/MIRParser/MILexer.h
> >  lib/CodeGen/MIRParser/MIParser.cpp
> >  lib/CodeGen/MIRPrinter.cpp
> >  test/CodeGen/MIR/X86/expected-register-after-flags.mir
> >  test/CodeGen/MIR/X86/implicit-register-flag.mir
> >  test/CodeGen/MIR/X86/machine-instructions.mir
> >  test/CodeGen/MIR/X86/register-mask-operands.mir
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D10709.28394.patch>
> > Index: lib/CodeGen/MIRParser/MILexer.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MILexer.cpp
> > +++ lib/CodeGen/MIRParser/MILexer.cpp
> > @@ -169,22 +171,20 @@
> >
> >    const auto &MCID = MF.getSubtarget().getInstrInfo()->get(OpCode);
> >
> > -  // Verify machine operands.
> > +  // Verify machine operands and determine if we should add the implicit
> > +  // register machine operands.
> > +  bool NoImplicit = false;
> >    if (!MCID.isVariadic()) {
> >      for (size_t I = 0, E = Operands.size(); I < E; ++I) {
> > +      if (Operands[I].isReg() && Operands[I].isImplicit())
> > +        NoImplicit = true;
> >        if (I < MCID.getNumOperands())
> >          continue;
> > -      // Mark this register as implicit to prevent an assertion when
> it's added
> > -      // to an instruction. This is a temporary workaround until the
> implicit
> > -      // register flag can be parsed.
> > -      if (Operands[I].isReg())
> > -        Operands[I].setImplicit();
> > +      // TODO: Check for extraneous machine operands.
> >      }
> >    }
> >
> > -  // TODO: Determine the implicit behaviour when implicit register
> flags are
> > -  // parsed.
> > -  auto *MI = MF.CreateMachineInstr(MCID, DebugLoc(),
> /*NoImplicit=*/true);
> > +  auto *MI = MF.CreateMachineInstr(MCID, DebugLoc(), NoImplicit);
>
> This all seems a little magical to me.  I'd rather, for now, be more
> conservative:
>
>   - Continue to pass `/* NoImplicit */ true`.
>   - Diagnostic (error? warning?) if the specified implicit operands
>     don't match the default ones for the opcode.  (Or does that get
>     caught by the machine verifier?)
>
> Or maybe this is the kind of thing a .mir file could opt into via an
> `autoimplicit` keyword somewhere, such as:
>
>     JG_1 %bb.2.exit, autoimplicit
>
> Thoughts from anyone else?
>

Thanks,
I more conservative approach works for me. I will post an updated patch
tomorrow.
We could also have a 'autoimplicit' property in the machine function YAML
mapping that would apply 'autoimplicit' to every machine instruction
automatically.


>
> >    for (const auto &Operand : Operands)
> >      MI->addOperand(MF, Operand);
> >    return MI;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150625/4efbef60/attachment.html>


More information about the llvm-commits mailing list