[PATCH] D75224: [SelectionDAG][FPEnv] Take into account SelectionDAG continuous CSE when setting the nofpexcept flag for constrained intrinsics

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: uweigand, spatel, cameron.mcinally, pengfei, andrew.w.kaylor.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

SelectionDAG CSEs nodes based on their result type and operands, but not their flags. The flags are expected to be intersected when they are CSEd. For FP nodes we manage both the fast math flags and the nofpexcept flag after the nodes have already been CSEd when they were created with getNode. The fast math flags we are managing correctly, but similar was not done for the nofpexcept flag.

Unfortunately, fixing this is a little involved because most constrained intrinsics are considered FPMathOperators, but not all. The check is based on whether they return an FP type or not. These nodes can have fast math flags. In order for the CSE detection to work we need to handle the fast math flags for the FPMathOperators and the nofpexcept flag for the constrained intrinsics that are FPMathOperators at the same time. This way there is only one call to set the flags per newly created node. So we need a separate block for the remaining intrinsics that aren't FPMathOperators.

We should probably make FPMathOperators also include all constrained intrinsics. But that requires making Operator.h include the intrinsic enums or IntrinsicInst.h which may expose other issues.


https://reviews.llvm.org/D75224

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/fp-intrinsics-flags.ll


Index: llvm/test/CodeGen/X86/fp-intrinsics-flags.ll
===================================================================
--- llvm/test/CodeGen/X86/fp-intrinsics-flags.ll
+++ llvm/test/CodeGen/X86/fp-intrinsics-flags.ll
@@ -108,7 +108,7 @@
 ; CHECK: [[MOV32rm:%[0-9]+]]:gr32 = MOV32rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.0)
 ; CHECK: [[MOV32rm1:%[0-9]+]]:gr32 = MOV32rm %fixed-stack.1, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.1, align 16)
 ; CHECK: [[MOVSDrm_alt:%[0-9]+]]:fr64 = MOVSDrm_alt %fixed-stack.3, 1, $noreg, 0, $noreg :: (load 8 from %fixed-stack.3, align 16)
-; CHECK: %3:fr64 = nofpexcept DIVSDrm [[MOVSDrm_alt]], %fixed-stack.2, 1, $noreg, 0, $noreg, implicit $mxcsr :: (load 8 from %fixed-stack.2)
+; CHECK: %3:fr64 = DIVSDrm [[MOVSDrm_alt]], %fixed-stack.2, 1, $noreg, 0, $noreg, implicit $mxcsr :: (load 8 from %fixed-stack.2)
 ; CHECK: MOVSDmr killed [[MOV32rm1]], 1, $noreg, 0, $noreg, %3 :: (store 8 into %ir.x, align 4)
 ; CHECK: MOVSDmr killed [[MOV32rm]], 1, $noreg, 0, $noreg, %3 :: (store 8 into %ir.y, align 4)
 ; CHECK: RET 0
@@ -126,7 +126,7 @@
 ; CHECK-LABEL: name: sitofp_cse
 ; CHECK: [[MOV32rm:%[0-9]+]]:gr32 = MOV32rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.0, align 8)
 ; CHECK: [[MOV32rm1:%[0-9]+]]:gr32 = MOV32rm %fixed-stack.1, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.1)
-; CHECK: %2:fr64 = nofpexcept CVTSI2SDrm %fixed-stack.2, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.2, align 16)
+; CHECK: %2:fr64 = CVTSI2SDrm %fixed-stack.2, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.2, align 16)
 ; CHECK: MOVSDmr killed [[MOV32rm1]], 1, $noreg, 0, $noreg, %2 :: (store 8 into %ir.x, align 4)
 ; CHECK: MOVSDmr killed [[MOV32rm]], 1, $noreg, 0, $noreg, %2 :: (store 8 into %ir.y, align 4)
 ; CHECK: RET 0
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1121,21 +1121,34 @@
     if (SDNode *Node = getNodeForIRValue(&I)) {
       SDNodeFlags IncomingFlags;
       IncomingFlags.copyFMF(*FPMO);
+
+      // Constrained FP intrinsics with fpexcept.ignore should also get
+      // the NoFPExcept flag.
+      if (auto *FPI = dyn_cast<ConstrainedFPIntrinsic>(&I))
+        if (FPI->getExceptionBehavior() == fp::ExceptionBehavior::ebIgnore)
+          IncomingFlags.setNoFPExcept(true);
+
+      if (!Node->getFlags().isDefined())
+        Node->setFlags(IncomingFlags);
+      else
+        Node->intersectFlagsWith(IncomingFlags);
+    }
+  } else if (auto *FPI = dyn_cast<ConstrainedFPIntrinsic>(&I)) {
+    // FIXME: Not every ConstrainedFPIntrinsic is a FPMathOperator.
+    if (SDNode *Node = getNodeForIRValue(&I)) {
+      SDNodeFlags IncomingFlags;
+
+      // Constrained FP intrinsics with fpexcept.ignore should also get
+      // the NoFPExcept flag.
+      if (FPI->getExceptionBehavior() == fp::ExceptionBehavior::ebIgnore)
+        IncomingFlags.setNoFPExcept(true);
+
       if (!Node->getFlags().isDefined())
         Node->setFlags(IncomingFlags);
       else
         Node->intersectFlagsWith(IncomingFlags);
     }
   }
-  // Constrained FP intrinsics with fpexcept.ignore should also get
-  // the NoFPExcept flag.
-  if (auto *FPI = dyn_cast<ConstrainedFPIntrinsic>(&I))
-    if (FPI->getExceptionBehavior() == fp::ExceptionBehavior::ebIgnore)
-      if (SDNode *Node = getNodeForIRValue(&I)) {
-        SDNodeFlags Flags = Node->getFlags();
-        Flags.setNoFPExcept(true);
-        Node->setFlags(Flags);
-      }
 
   if (!I.isTerminator() && !HasTailCall &&
       !isStatepoint(&I)) // statepoints handle their exports internally


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75224.246865.patch
Type: text/x-patch
Size: 3775 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200227/c0c6295c/attachment.bin>