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

[Duncan P. N. Exon Smith]

> 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?

>    for (const auto &Operand : Operands)
>      MI->addOperand(MF, Operand);
>    return MI;