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

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

That would create a circular dependence.

[arsenm]

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

828–834

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

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

assert(NumMIOperands >= II.getNumOperands() &&
            "#operands for dag node doesn't match .td file!");
 #endif
 
   // Create the new machine instruction.
   MachineInstrBuilder MIB = BuildMI(*MF, Node->getDebugLoc(), II);
 
   // Add result register values for things that are defined by this
   // instruction.
-  if (NumResults)
+  if (NumResults) {
     CreateVirtualRegisters(Node, MIB, II, IsClone, IsCloned, VRBaseMap);
 
+    // Transfer any IR flags from the SDNode to the MachineInstr
+    MachineInstr *MI = MIB.getInstr();
+    const SDNodeFlags Flags = Node->getFlags();
+    if (Flags.hasNoSignedZeros())
+      MI->setFlag(MachineInstr::MIFlag::FmNsz);
+
+    if (Flags.hasAllowReciprocal())
+      MI->setFlag(MachineInstr::MIFlag::FmArcp);
+
+    if (Flags.hasNoNaNs())
+      MI->setFlag(MachineInstr::MIFlag::FmNoNans);
+
+    if (Flags.hasNoInfs())
+      MI->setFlag(MachineInstr::MIFlag::FmNoInfs);
+
+    if (Flags.hasAllowContract())
+      MI->setFlag(MachineInstr::MIFlag::FmContract);
+
+    if (Flags.hasAproximateFuncs())
+      MI->setFlag(MachineInstr::MIFlag::FmAfn);
+
+    if (Flags.hasAllowReassociation())
+      MI->setFlag(MachineInstr::MIFlag::FmReassoc);
+  }
+
   // Emit all of the actual operands of this instruction, adding them to the
   // instruction as appropriate.
   bool HasOptPRefs = NumDefs > NumResults;
   assert((!HasOptPRefs || !HasPhysRegOuts) &&
          "Unable to cope with optional defs and phys regs defs!");
   unsigned NumSkip = HasOptPRefs ? NumDefs - NumResults : 0;
   for (unsigned i = NumSkip; i != NodeOperands; ++i)
     AddOperand(MIB, Node->getOperand(i), i-NumSkip+NumDefs, &II,