Mapping SDNode flags to MachineInstr flags

[mcberg2017]

Summary

Providing the glue to map SDNode fast math sub flags to MachineInstr fast math sub flags.

[spatel]

The transfer itself is straightforward, so LGTM.

But if you can explain here for the record how we get from the IR FMF and function attributes to the DAG flags, that would be good. (And yes, I know you have some of the patches in progress to improve this.)

[mcberg2017]

As part to the global explanation behind the design of this and other components in this stream of reviews, fast math flags are mined directly off of IR and propagated (or will be more so incrementally over time), so that we can rely on similar functionality without the use of the module level unsafe flag. The goal is to achieve parity in code generation and to propagate context sensitive guards over optimization and code generation so that relative functional IR attributes control these features. As in the summary this patch provides the mapping off of SDNode into MachineInstr so that we carry full context at the IR level from high level compilation into back ends.

[spatel]

In D46447#1088568, @mcberg2017 wrote:

As part to the global explanation behind the design of this and other components in this stream of reviews, fast math flags are mined directly off of IR and propagated (or will be more so incrementally over time), so that we can rely on similar functionality without the use of the module level unsafe flag. The goal is to achieve parity in code generation and to propagate context sensitive guards over optimization and code generation so that relative functional IR attributes control these features. As in the summary this patch provides the mapping off of SDNode into MachineInstr so that we carry full context at the IR level from high level compilation into back ends.

I was actually more concerned with the non-obvious outcome in the affected regression tests because this particular case (sqrt estimate) just came up in:
https://bugs.llvm.org/show_bug.cgi?id=37344

In the 1st test (plain sqrt estimate), there's only a function attribute. The estimate transform is predicated on that function attribute and then applies 'reassoc' to new nodes as much as it could at the time it was implemented. That means it set that flag for binops (fadd/fmul), but not fma or rsqrt!

In the 2nd test (reciprocal sqrt estimate), the fdiv is marked 'fast', so the transform is able to propagate that to all of the created binop nodes (again, the 'fma' get nothing currently, but they should).

In summary, it's full of bugs.

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.hasApproximateFuncs())
+      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,

test/CodeGen/X86/sqrt-fastmath-mir.ll

 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx2,fma -stop-after=expand-isel-pseudos 2>&1 | FileCheck %s
 
 declare float @llvm.sqrt.f32(float) #0
 
 define float @foo(float %f) #0 {
 ; CHECK: {{name: *foo}}
 ; CHECK: body:
 ; CHECK:     %0:fr32 = COPY $xmm0
 ; CHECK:     %1:fr32 = VRSQRTSSr killed %2, %0
-; CHECK:     %3:fr32 = VMULSSrr %0, %1
+; CHECK:     %3:fr32 = reassoc VMULSSrr %0, %1
 ; CHECK:     %4:fr32 = VMOVSSrm
 ; CHECK:     %5:fr32 = VFMADD213SSr %1, killed %3, %4
 ; CHECK:     %6:fr32 = VMOVSSrm
-; CHECK:     %7:fr32 = VMULSSrr %1, %6
-; CHECK:     %8:fr32 = VMULSSrr killed %7, killed %5
-; CHECK:     %9:fr32 = VMULSSrr %0, %8
+; CHECK:     %7:fr32 = reassoc VMULSSrr %1, %6
+; CHECK:     %8:fr32 = reassoc VMULSSrr killed %7, killed %5
+; CHECK:     %9:fr32 = reassoc VMULSSrr %0, %8
 ; CHECK:     %10:fr32 = VFMADD213SSr %8, %9, %4
-; CHECK:     %11:fr32 = VMULSSrr %9, %6
-; CHECK:     %12:fr32 = VMULSSrr killed %11, killed %10
+; CHECK:     %11:fr32 = reassoc VMULSSrr %9, %6
+; CHECK:     %12:fr32 = reassoc VMULSSrr killed %11, killed %10
 ; CHECK:     %14:fr32 = FsFLD0SS
 ; CHECK:     %15:fr32 = VCMPSSrr %0, killed %14, 0
 ; CHECK:     %17:vr128 = VANDNPSrr killed %16, killed %13
 ; CHECK:     $xmm0 = COPY %18
 ; CHECK:     RET 0, $xmm0
   %call = tail call float @llvm.sqrt.f32(float %f) #1
   ret float %call
 }
 
 define float @rfoo(float %f) #0 {
 ; CHECK: {{name: *rfoo}}
 ; CHECK: body:             |
 ; CHECK:     %0:fr32 = COPY $xmm0
 ; CHECK:     %1:fr32 = VRSQRTSSr killed %2, %0
-; CHECK:     %3:fr32 = VMULSSrr %0, %1
+; CHECK:     %3:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr %0, %1
 ; CHECK:     %4:fr32 = VMOVSSrm
 ; CHECK:     %5:fr32 = VFMADD213SSr %1, killed %3, %4
 ; CHECK:     %6:fr32 = VMOVSSrm
-; CHECK:     %7:fr32 = VMULSSrr %1, %6
-; CHECK:     %8:fr32 = VMULSSrr killed %7, killed %5
-; CHECK:     %9:fr32 = VMULSSrr %0, %8
+; CHECK:     %7:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr %1, %6
+; CHECK:     %8:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr killed %7, killed %5
+; CHECK:     %9:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr %0, %8
 ; CHECK:     %10:fr32 = VFMADD213SSr %8, killed %9, %4
-; CHECK:     %11:fr32 = VMULSSrr %8, %6
-; CHECK:     %12:fr32 = VMULSSrr killed %11, killed %10
+; CHECK:     %11:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr %8, %6
+; CHECK:     %12:fr32 = nnan ninf nsz arcp contract afn reassoc VMULSSrr killed %11, killed %10
 ; CHECK:     $xmm0 = COPY %12
 ; CHECK:     RET 0, $xmm0
   %sqrt = tail call float @llvm.sqrt.f32(float %f)
   %div = fdiv fast float 1.0, %sqrt
   ret float %div
 }
 
 attributes #0 = { "unsafe-fp-math"="true" "reciprocal-estimates"="sqrt:2" }
 attributes #1 = { nounwind readnone }