MachineInst support mapping SDNode fast math flags for support in Back End code generation

[mcberg2017]

Summary

Machine Instruction flags for fast math support and MIR print support

[mcberg2017]

include/llvm/CodeGen/MachineInstr.h

110

Does this change the overall size of the MachineInstr struct, or is there already enough padding to use?

[mcberg2017]

include/llvm/CodeGen/MachineInstr.h

110

The padding in the MIFlag enum is such that the new flags do not change the size of the class, however, the Flags field changes it by 1 byte in size.

[arsenm]

This is missing the support and tests in the MIR printer for these

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

828–834 ↗

(On Diff #142973)

This section belongs with the DAG patch

[mcberg2017]

Meantime, I will look into the tests and MIR printer changes.

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

828–834 ↗

(On Diff #142973)

That would create a circular dependence.

[arsenm]

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

828–834 ↗

(On Diff #142973)

How? You should be able to make all the MI changes first, independent of the DAG

[mcberg2017]

As per request, I have moved lib/CodeGen/MachineInstr.cpp to D45710, also added printer code for MI. Tests were requested as well, however they will appear with all the other functional aspects of these changes in D45710, as the full set of changes is needed for test modifications.

[arsenm]

What other changes are necessary? This should be sufficient to test the MIR parser

[mcberg2017]

It should be noted with the changes above, that this change list is now required to check in first before D45710 and that there is a hard dependency on this code.

[mcberg2017]

The code in lib/CodeGen/SelectionDAG/InstrEmitter.cpp sets the flags, which will shortly be D45710, without that, nothing changes in the mir tests, ergo the check component testing for the fast math flags will cause modified tests which run through the MIR printer to fail until those changes are checked in.

[arsenm]

In D45781#1077386, @mcberg2017 wrote:

The code in lib/CodeGen/SelectionDAG/InstrEmitter.cpp sets the flags, which will shortly be D45710, without that, nothing changes in the mir tests, ergo the check component testing for the fast math flags will cause modified tests which run through the MIR printer to fail until those changes are checked in.

If the instructions are codegened from SelectionDAG. This seems to be missing the part to parse these names and set the flags in the MIR parser, which is what should be setting the flags here.

[mcberg2017]

Ok, I will add the parser/lexer pieces to this review, once I have them authored/tested.

[mcberg2017]

Added mir synchronization support for MI fast math flags with a test.

[mcberg2017]

Are there any outstanding issues regarding these changes? Does it look ok for approval?

[arsenm]

test/CodeGen/X86/sqrt-fastmath.mir

1 ↗

(On Diff #143855)

This test belongs in test/CodeGen/MIR/X86. The sqrt also isn't needed in the name, just fast-math-flags would be better

8–12 ↗

(On Diff #143855)

Don't need the IR section

16–17 ↗

(On Diff #143855)

You can strip out most of this stuff

74–75 ↗

(On Diff #143855)

This doesn't need to be a meaningful function. You can just have one sample instruction for each individual flag, and maybe a few with combination flags for good measure

[mcberg2017]

MIR test adjusted, please have a look.

[arsenm]

LGTM

include/llvm/CodeGen/MachineInstr.h

public:
 
   enum MIFlag {
     NoFlags      = 0,
     FrameSetup   = 1 << 0,              // Instruction is used as a part of
                                         // function frame setup code.
     FrameDestroy = 1 << 1,              // Instruction is used as a part of
                                         // function frame destruction code.
     BundledPred  = 1 << 2,              // Instruction has bundled predecessors.
-    BundledSucc  = 1 << 3               // Instruction has bundled successors.
+    BundledSucc  = 1 << 3,              // Instruction has bundled successors.
+    FmNoNans     = 1 << 4,              // Instruction does not support Fast
+                                        // math nan values.
+    FmNoInfs     = 1 << 5,              // Instruction does not support Fast
+                                        // math infinity values.
+    FmNsz        = 1 << 6,              // Instruction is not required to retain
+                                        // signed zero values.
+    FmArcp       = 1 << 7,              // Instruction supports Fast math
+                                        // reciprocal approximations.
+    FmContract   = 1 << 8,              // Instruction supports Fast math
+                                        // contraction operations like fma.
+    FmAfn        = 1 << 9,              // Instruction may map to Fast math
+                                        // instrinsic approximation.
+    FmReassoc    = 1 << 10              // Instruction supports Fast math
+                                        // reassociation of operand order.
   };
 
 private:
   const MCInstrDesc *MCID;              // Instruction descriptor.
   MachineBasicBlock *Parent = nullptr;  // Pointer to the owning basic block.
 
   // Operands are allocated by an ArrayRecycler.
   MachineOperand *Operands = nullptr;   // Pointer to the first operand.
   unsigned NumOperands = 0;             // Number of operands on instruction.
   using OperandCapacity = ArrayRecycler<MachineOperand>::Capacity;
   OperandCapacity CapOperands;          // Capacity of the Operands array.
 
-  uint8_t Flags = 0;                    // Various bits of additional
+  uint16_t Flags = 0;                   // Various bits of additional
                                         // information about machine
                                         // instruction.
 
   uint8_t AsmPrinterFlags = 0;          // Various bits of information used by
                                         // the AsmPrinter to emit helpful
                                         // comments.  This is *not* semantic
                                         // information.  Do not use this for
                                         // anything other than to convey comment
public:
 
   /// Return whether an MI flag is set.
   bool getFlag(MIFlag Flag) const {
     return Flags & Flag;
   }
 
   /// Set a MI flag.
   void setFlag(MIFlag Flag) {
-    Flags |= (uint8_t)Flag;
+    Flags |= (uint16_t)Flag;
   }
 
   void setFlags(unsigned flags) {
     // Filter out the automatically maintained flags.
     unsigned Mask = BundledPred | BundledSucc;
     Flags = (Flags & Mask) | (flags & ~Mask);
   }
 
   /// clearFlag - Clear a MI flag.
   void clearFlag(MIFlag Flag) {
-    Flags &= ~((uint8_t)Flag);
+    Flags &= ~((uint16_t)Flag);
   }
 
   /// Return true if MI is in a bundle (but not the first MI in a bundle).
   ///
   /// A bundle looks like this before it's finalized:
   ///   ----------------
   ///   |      MI      |
   ///   ----------------

lib/CodeGen/MIRParser/MILexer.h

enum TokenKind {
     kw_undef,
     kw_internal,
     kw_early_clobber,
     kw_debug_use,
     kw_renamable,
     kw_tied_def,
     kw_frame_setup,
     kw_frame_destroy,
+    kw_nnan,
+    kw_ninf,
+    kw_nsz,
+    kw_arcp,
+    kw_contract,
+    kw_afn,
+    kw_reassoc,
     kw_debug_location,
     kw_cfi_same_value,
     kw_cfi_offset,
     kw_cfi_rel_offset,
     kw_cfi_def_cfa_register,
     kw_cfi_def_cfa_offset,
     kw_cfi_adjust_cfa_offset,
     kw_cfi_escape,

lib/CodeGen/MIRParser/MILexer.cpp

return StringSwitch<MIToken::TokenKind>(Identifier)
       .Case("undef", MIToken::kw_undef)
       .Case("internal", MIToken::kw_internal)
       .Case("early-clobber", MIToken::kw_early_clobber)
       .Case("debug-use", MIToken::kw_debug_use)
       .Case("renamable", MIToken::kw_renamable)
       .Case("tied-def", MIToken::kw_tied_def)
       .Case("frame-setup", MIToken::kw_frame_setup)
       .Case("frame-destroy", MIToken::kw_frame_destroy)
+      .Case("nnan", MIToken::kw_nnan)
+      .Case("ninf", MIToken::kw_ninf)
+      .Case("nsz", MIToken::kw_nsz)
+      .Case("arcp", MIToken::kw_arcp)
+      .Case("contract", MIToken::kw_contract)
+      .Case("afn", MIToken::kw_afn)
+      .Case("reassoc", MIToken::kw_reassoc)
       .Case("debug-location", MIToken::kw_debug_location)
       .Case("same_value", MIToken::kw_cfi_same_value)
       .Case("offset", MIToken::kw_cfi_offset)
       .Case("rel_offset", MIToken::kw_cfi_rel_offset)
       .Case("def_cfa_register", MIToken::kw_cfi_def_cfa_register)
       .Case("def_cfa_offset", MIToken::kw_cfi_def_cfa_offset)
       .Case("adjust_cfa_offset", MIToken::kw_cfi_adjust_cfa_offset)
       .Case("escape", MIToken::kw_cfi_escape)

lib/CodeGen/MIRParser/MIParser.cpp

return error(Operands.empty() ? Token.location() : Operands.back().End,
                  Twine("missing implicit register operand '") +
                      printImplicitRegisterFlag(I) + " %" +
                      getRegisterName(TRI, I.getReg()) + "'");
   }
   return false;
 }
 
 bool MIParser::parseInstruction(unsigned &OpCode, unsigned &Flags) {
-  // Allow both:
-  // * frame-setup frame-destroy OPCODE
-  // * frame-destroy frame-setup OPCODE
+  // Allow frame and fast math flags for OPCODE
   while (Token.is(MIToken::kw_frame_setup) ||
-         Token.is(MIToken::kw_frame_destroy)) {
-    Flags |= Token.is(MIToken::kw_frame_setup) ? MachineInstr::FrameSetup
-                                               : MachineInstr::FrameDestroy;
+         Token.is(MIToken::kw_frame_destroy) ||
+         Token.is(MIToken::kw_nnan) ||
+         Token.is(MIToken::kw_ninf) ||
+         Token.is(MIToken::kw_nsz) ||
+         Token.is(MIToken::kw_arcp) ||
+         Token.is(MIToken::kw_contract) ||
+         Token.is(MIToken::kw_afn) ||
+         Token.is(MIToken::kw_reassoc)) {
+    // Mine frame and fast math flags
+    if (Token.is(MIToken::kw_frame_setup))
+      Flags |= MachineInstr::FrameSetup;
+    if (Token.is(MIToken::kw_frame_destroy))
+      Flags |= MachineInstr::FrameDestroy;
+    if (Token.is(MIToken::kw_nnan))
+      Flags |= MachineInstr::FmNoNans;
+    if (Token.is(MIToken::kw_ninf))
+      Flags |= MachineInstr::FmNoInfs;
+    if (Token.is(MIToken::kw_nsz))
+      Flags |= MachineInstr::FmNsz;
+    if (Token.is(MIToken::kw_arcp))
+      Flags |= MachineInstr::FmArcp;
+    if (Token.is(MIToken::kw_contract))
+      Flags |= MachineInstr::FmContract;
+    if (Token.is(MIToken::kw_afn))
+      Flags |= MachineInstr::FmAfn;
+    if (Token.is(MIToken::kw_reassoc))
+      Flags |= MachineInstr::FmReassoc;
+
     lex();
   }
   if (Token.isNot(MIToken::Identifier))
     return error("expected a machine instruction");
   StringRef InstrName = Token.stringValue();
   if (parseInstrName(InstrName, OpCode))
     return error(Twine("unknown machine instruction name '") + InstrName + "'");
   lex();

lib/CodeGen/MIRPrinter.cpp

void MIPrinter::print(const MachineInstr &MI) {
   }
 
   if (I)
     OS << " = ";
   if (MI.getFlag(MachineInstr::FrameSetup))
     OS << "frame-setup ";
   if (MI.getFlag(MachineInstr::FrameDestroy))
     OS << "frame-destroy ";
+  if (MI.getFlag(MachineInstr::FmNoNans))
+    OS << "nnan ";
+  if (MI.getFlag(MachineInstr::FmNoInfs))
+    OS << "ninf ";
+  if (MI.getFlag(MachineInstr::FmNsz))
+    OS << "nsz ";
+  if (MI.getFlag(MachineInstr::FmArcp))
+    OS << "arcp ";
+  if (MI.getFlag(MachineInstr::FmContract))
+    OS << "contract ";
+  if (MI.getFlag(MachineInstr::FmAfn))
+    OS << "afn ";
+  if (MI.getFlag(MachineInstr::FmReassoc))
+    OS << "reassoc ";
 
   OS << TII->getName(MI.getOpcode());
   if (I < E)
     OS << ' ';
 
   bool NeedComma = false;
   for (; I < E; ++I) {
     if (NeedComma)

lib/CodeGen/MachineInstr.cpp

void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
 
   if (StartOp != 0)
     OS << " = ";
 
   if (getFlag(MachineInstr::FrameSetup))
     OS << "frame-setup ";
   if (getFlag(MachineInstr::FrameDestroy))
     OS << "frame-destroy ";
+  if (getFlag(MachineInstr::FmNoNans))
+    OS << "nnan ";
+  if (getFlag(MachineInstr::FmNoInfs))
+    OS << "ninf ";
+  if (getFlag(MachineInstr::FmNsz))
+    OS << "nsz ";
+  if (getFlag(MachineInstr::FmArcp))
+    OS << "arcp ";
+  if (getFlag(MachineInstr::FmContract))
+    OS << "contract ";
+  if (getFlag(MachineInstr::FmAfn))
+    OS << "afn ";
+  if (getFlag(MachineInstr::FmReassoc))
+    OS << "reassoc ";
 
   // Print the opcode name.
   if (TII)
     OS << TII->getName(getOpcode());
   else
     OS << "UNKNOWN";
 
   if (SkipOpers)

test/CodeGen/MIR/X86/fastmath.mir

+# RUN: llc -march=x86-64 -run-pass none -o - %s | FileCheck %s
+# This test ensures that the MIR parser parses the fast math instruction flags.
+
+...
+---
+name:            baz
+body: |
+  bb.0.entry:
+    liveins: $xmm0
+  
+    ; CHECK:     %0:fr32 = COPY $xmm0
+    %0:fr32 = COPY $xmm0
+    ; CHECK:     %1:fr32 = nnan VMULSSrr %0, %0
+    %1:fr32 = nnan VMULSSrr %0, %0
+    ; CHECK:     %2:fr32 = ninf VMULSSrr %1, %1
+    %2:fr32 = ninf VMULSSrr %1, %1
+    ; CHECK:     %3:fr32 = nsz VMULSSrr %2, %2
+    %3:fr32 = nsz VMULSSrr %2, %2
+    ; CHECK:     %4:fr32 = arcp VMULSSrr %3, %3
+    %4:fr32 = arcp VMULSSrr %3, %3
+    ; CHECK:     %5:fr32 = contract VMULSSrr %4, %4
+    %5:fr32 = contract VMULSSrr %4, %4
+    ; CHECK:     %6:fr32 = afn VMULSSrr %5, %5
+    %6:fr32 = afn VMULSSrr %5, %5
+    ; CHECK:     %7:fr32 = reassoc VMULSSrr %6, %6
+    %7:fr32 = reassoc VMULSSrr %6, %6
+    ; CHECK:     %8:fr32 = nsz arcp contract afn reassoc VMULSSrr %7, %7
+    %8:fr32 = nsz arcp contract afn reassoc VMULSSrr %7, %7
+    ; CHECK:     %9:fr32 = contract afn reassoc VMULSSrr %8, %8
+    %9:fr32 = contract afn reassoc VMULSSrr %8, %8
+    ; CHECK:     $xmm0 = COPY %9
+    $xmm0 = COPY %9
+    ; CHECK:     RET 0, $xmm0
+    RET 0, $xmm0
+
+...