[InstCombine] Preserve fast math flags when combining PHIs

[sbaranga]

Summary

When optimizing PHIs which have inputs floating point binary
operators, we preserve all IR flags except the fast math
flags.

This change removes the logic which tracked some of the IR flags
(no wrap, exact) and replaces it by doing an and on the IR flags of
all inputs to the PHI - which will also handle the fast math
flags.

[majnemer]

Nice, LGTM with nits.

test/Transforms/InstCombine/phi-preserve-ir-flags.ll

5

I'd add a little more for the labels, something like:

; CHECK-LABEL: define float @f(

[sbaranga]

Check a longer string when performing label checks.

[sbaranga]

Thanks! Committed in r267139

lib/Transforms/InstCombine/InstCombinePHI.cpp

Instruction *InstCombiner::FoldPHIArgBinOpIntoPHI(PHINode &PN) {
   assert(isa<BinaryOperator>(FirstInst) || isa<CmpInst>(FirstInst));
   unsigned Opc = FirstInst->getOpcode();
   Value *LHSVal = FirstInst->getOperand(0);
   Value *RHSVal = FirstInst->getOperand(1);
 
   Type *LHSType = LHSVal->getType();
   Type *RHSType = RHSVal->getType();
 
-  bool isNUW = false, isNSW = false, isExact = false;
-  if (OverflowingBinaryOperator *BO =
-        dyn_cast<OverflowingBinaryOperator>(FirstInst)) {
-    isNUW = BO->hasNoUnsignedWrap();
-    isNSW = BO->hasNoSignedWrap();
-  } else if (PossiblyExactOperator *PEO =
-               dyn_cast<PossiblyExactOperator>(FirstInst))
-    isExact = PEO->isExact();
-
   // Scan to see if all operands are the same opcode, and all have one use.
   for (unsigned i = 1; i != PN.getNumIncomingValues(); ++i) {
     Instruction *I = dyn_cast<Instruction>(PN.getIncomingValue(i));
     if (!I || I->getOpcode() != Opc || !I->hasOneUse() ||
         // Verify type of the LHS matches so we don't fold cmp's of different
         // types.
         I->getOperand(0)->getType() != LHSType ||
         I->getOperand(1)->getType() != RHSType)
       return nullptr;
 
     // If they are CmpInst instructions, check their predicates
     if (CmpInst *CI = dyn_cast<CmpInst>(I))
       if (CI->getPredicate() != cast<CmpInst>(FirstInst)->getPredicate())
         return nullptr;
 
-    if (isNUW)
-      isNUW = cast<OverflowingBinaryOperator>(I)->hasNoUnsignedWrap();
-    if (isNSW)
-      isNSW = cast<OverflowingBinaryOperator>(I)->hasNoSignedWrap();
-    if (isExact)
-      isExact = cast<PossiblyExactOperator>(I)->isExact();
-
     // Keep track of which operand needs a phi node.
     if (I->getOperand(0) != LHSVal) LHSVal = nullptr;
     if (I->getOperand(1) != RHSVal) RHSVal = nullptr;
   }
 
   // If both LHS and RHS would need a PHI, don't do this transformation,
   // because it would increase the number of PHIs entering the block,
   // which leads to higher register pressure. This is especially
CmpInst *NewCI = CmpInst::Create(CIOp->getOpcode(), CIOp->getPredicate(),
                                      LHSVal, RHSVal);
     NewCI->setDebugLoc(FirstInst->getDebugLoc());
     return NewCI;
   }
 
   BinaryOperator *BinOp = cast<BinaryOperator>(FirstInst);
   BinaryOperator *NewBinOp =
     BinaryOperator::Create(BinOp->getOpcode(), LHSVal, RHSVal);
-  if (isNUW) NewBinOp->setHasNoUnsignedWrap();
-  if (isNSW) NewBinOp->setHasNoSignedWrap();
-  if (isExact) NewBinOp->setIsExact();
+
+  NewBinOp->copyIRFlags(PN.getIncomingValue(0));
+
+  for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i)
+    NewBinOp->andIRFlags(PN.getIncomingValue(i));
+
   NewBinOp->setDebugLoc(FirstInst->getDebugLoc());
   return NewBinOp;
 }
 
 Instruction *InstCombiner::FoldPHIArgGEPIntoPHI(PHINode &PN) {
   GetElementPtrInst *FirstInst =cast<GetElementPtrInst>(PN.getIncomingValue(0));
 
   SmallVector<Value*, 16> FixedOperands(FirstInst->op_begin(),
if (isa<LoadInst>(FirstInst))
     return FoldPHIArgLoadIntoPHI(PN);
 
   // Scan the instruction, looking for input operations that can be folded away.
   // If all input operands to the phi are the same instruction (e.g. a cast from
   // the same type or "+42") we can pull the operation through the PHI, reducing
   // code size and simplifying code.
   Constant *ConstantOp = nullptr;
   Type *CastSrcTy = nullptr;
-  bool isNUW = false, isNSW = false, isExact = false;
 
   if (isa<CastInst>(FirstInst)) {
     CastSrcTy = FirstInst->getOperand(0)->getType();
 
     // Be careful about transforming integer PHIs.  We don't want to pessimize
     // the code by turning an i32 into an i1293.
     if (PN.getType()->isIntegerTy() && CastSrcTy->isIntegerTy()) {
       if (!ShouldChangeType(PN.getType(), CastSrcTy))
         return nullptr;
     }
   } else if (isa<BinaryOperator>(FirstInst) || isa<CmpInst>(FirstInst)) {
     // Can fold binop, compare or shift here if the RHS is a constant,
     // otherwise call FoldPHIArgBinOpIntoPHI.
     ConstantOp = dyn_cast<Constant>(FirstInst->getOperand(1));
     if (!ConstantOp)
       return FoldPHIArgBinOpIntoPHI(PN);
-
-    if (OverflowingBinaryOperator *BO =
-        dyn_cast<OverflowingBinaryOperator>(FirstInst)) {
-      isNUW = BO->hasNoUnsignedWrap();
-      isNSW = BO->hasNoSignedWrap();
-    } else if (PossiblyExactOperator *PEO =
-               dyn_cast<PossiblyExactOperator>(FirstInst))
-      isExact = PEO->isExact();
   } else {
     return nullptr;  // Cannot fold this operation.
   }
 
   // Check to see if all arguments are the same operation.
   for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i) {
     Instruction *I = dyn_cast<Instruction>(PN.getIncomingValue(i));
     if (!I || !I->hasOneUse() || !I->isSameOperationAs(FirstInst))
       return nullptr;
     if (CastSrcTy) {
       if (I->getOperand(0)->getType() != CastSrcTy)
         return nullptr;  // Cast operation must match.
     } else if (I->getOperand(1) != ConstantOp) {
       return nullptr;
     }
-
-    if (isNUW)
-      isNUW = cast<OverflowingBinaryOperator>(I)->hasNoUnsignedWrap();
-    if (isNSW)
-      isNSW = cast<OverflowingBinaryOperator>(I)->hasNoSignedWrap();
-    if (isExact)
-      isExact = cast<PossiblyExactOperator>(I)->isExact();
   }
 
   // Okay, they are all the same operation.  Create a new PHI node of the
   // correct type, and PHI together all of the LHS's of the instructions.
   PHINode *NewPN = PHINode::Create(FirstInst->getOperand(0)->getType(),
                                    PN.getNumIncomingValues(),
                                    PN.getName()+".in");
 
if (CastInst *FirstCI = dyn_cast<CastInst>(FirstInst)) {
     CastInst *NewCI = CastInst::Create(FirstCI->getOpcode(), PhiVal,
                                        PN.getType());
     NewCI->setDebugLoc(FirstInst->getDebugLoc());
     return NewCI;
   }
 
   if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(FirstInst)) {
     BinOp = BinaryOperator::Create(BinOp->getOpcode(), PhiVal, ConstantOp);
-    if (isNUW) BinOp->setHasNoUnsignedWrap();
-    if (isNSW) BinOp->setHasNoSignedWrap();
-    if (isExact) BinOp->setIsExact();
+    BinOp->copyIRFlags(PN.getIncomingValue(0));
+
+    for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i)
+      BinOp->andIRFlags(PN.getIncomingValue(i));
+
     BinOp->setDebugLoc(FirstInst->getDebugLoc());
     return BinOp;
   }
 
   CmpInst *CIOp = cast<CmpInst>(FirstInst);
   CmpInst *NewCI = CmpInst::Create(CIOp->getOpcode(), CIOp->getPredicate(),
                                    PhiVal, ConstantOp);
   NewCI->setDebugLoc(FirstInst->getDebugLoc());

test/Transforms/InstCombine/phi-preserve-ir-flags.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+; CHECK-LABEL: f
+define float @f(float %a, float %b, float %c, i1 %cond) {
+entry:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %sub0 = fsub fast float %a, %b
+  br label %cond.end
+
+cond.false:
+  %sub1 = fsub fast float %a, %c
+  br label %cond.end
+
+; The fast-math flags should always be transfered if possible.
+; CHECK-LABEL: cond.end
+; CHECK  [[PHI:%[^ ]*]] = phi float [ %b, %cond.true ], [ %c, %cond.false ]
+; CHECK  fsub fast float %a, [[PHI]]
+cond.end:
+  %e = phi float [ %sub0, %cond.true ], [ %sub1, %cond.false ]
+  ret float %e
+}
+
+; CHECK-LABEL: g
+define float @g(float %a, float %b, float %c, i1 %cond) {
+entry:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %sub0 = fsub fast float %a, %b
+  br label %cond.end
+
+cond.false:
+  %sub1 = fsub float %a, %c
+  br label %cond.end
+
+; The fast-math flags should always be transfered if possible.
+; CHECK-LABEL: cond.end
+; CHECK  [[PHI:%[^ ]*]] = phi float [ %b, %cond.true ], [ %c, %cond.false ]
+; CHECK  fsub float %a, [[PHI]]
+cond.end:
+  %e = phi float [ %sub0, %cond.true ], [ %sub1, %cond.false ]
+  ret float %e
+}
+
+; CHECK-LABEL: h
+define float @h(float %a, float %b, float %c, i1 %cond) {
+entry:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %sub0 = fsub fast float %a, 2.0
+  br label %cond.end
+
+cond.false:
+  %sub1 = fsub fast float %b, 2.0
+  br label %cond.end
+
+; CHECK-LABEL: cond.end
+; CHECK  [[PHI:%[^ ]*]] = phi float [ %a, %cond.true ], [ %b, %cond.false ]
+; CHECK  fadd fast float %a, [[PHI]]
+cond.end:
+  %e = phi float [ %sub0, %cond.true ], [ %sub1, %cond.false ]
+  ret float %e
+}
+
+; CHECK-LABEL: i
+define float @i(float %a, float %b, float %c, i1 %cond) {
+entry:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %sub0 = fsub fast float %a, 2.0
+  br label %cond.end
+
+cond.false:
+  %sub1 = fsub float %b, 2.0
+  br label %cond.end
+
+; CHECK-LABEL: cond.end
+; CHECK  [[PHI:%[^ ]*]] = phi float [ %a, %cond.true ], [ %b, %cond.false ]
+; CHECK  fadd float %a, [[PHI]]
+cond.end:
+  %e = phi float [ %sub0, %cond.true ], [ %sub1, %cond.false ]
+  ret float %e
+}