[llvm] r354689 - [CGP] move overflow intrinsic insertion to common location; NFCI

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Fri Feb 22 12:20:24 2019
New Revision: 354689

URL: http://llvm.org/viewvc/llvm-project?rev=354689&view=rev
Log:
[CGP] move overflow intrinsic insertion to common location; NFCI

We need to enhance the uaddo matching to handle special-cases
as seen in PR40486 and PR31754. That means we won't necessarily
have a def-use pattern, so we'll need to check dominance to
determine where to place the intrinsic (as we already do for
usubo). This preliminary patch is just rearranging the code,
so the planned follow-up to improve uaddo will be more clear.

Modified:
    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=354689&r1=354688&r2=354689&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Fri Feb 22 12:20:24 2019
@@ -1159,19 +1159,33 @@ static bool OptimizeNoopCopyExpression(C
   return SinkCast(CI);
 }
 
-static void replaceMathCmpWithIntrinsic(BinaryOperator *BO, CmpInst *Cmp,
-                                        Instruction *InsertPt,
+static bool replaceMathCmpWithIntrinsic(BinaryOperator *BO, CmpInst *Cmp,
                                         Intrinsic::ID IID) {
+  // We allow matching the canonical IR (add X, C) back to (usubo X, -C).
   Value *Arg0 = BO->getOperand(0);
   Value *Arg1 = BO->getOperand(1);
-
-  // We allow matching the canonical IR (add X, C) back to (usubo X, -C).
   if (BO->getOpcode() == Instruction::Add &&
       IID == Intrinsic::usub_with_overflow) {
     assert(isa<Constant>(Arg1) && "Unexpected input for usubo");
     Arg1 = ConstantExpr::getNeg(cast<Constant>(Arg1));
   }
 
+  Instruction *InsertPt;
+  if (BO->hasOneUse() && BO->user_back() == Cmp) {
+    // If the math is only used by the compare, insert at the compare to keep
+    // the condition in the same block as its users. (CGP aggressively sinks
+    // compares to help out SDAG.)
+    InsertPt = Cmp;
+  } else {
+    // The math and compare may be independent instructions. Check dominance to
+    // determine the insertion point for the intrinsic.
+    DominatorTree DT(*BO->getFunction());
+    bool MathDominates = DT.dominates(BO, Cmp);
+    if (!MathDominates && !DT.dominates(Cmp, BO))
+      return false;
+    InsertPt = MathDominates ? cast<Instruction>(BO) : cast<Instruction>(Cmp);
+  }
+
   IRBuilder<> Builder(InsertPt);
   Value *MathOV = Builder.CreateBinaryIntrinsic(IID, Arg0, Arg1);
   Value *Math = Builder.CreateExtractValue(MathOV, 0, "math");
@@ -1180,12 +1194,13 @@ static void replaceMathCmpWithIntrinsic(
   Cmp->replaceAllUsesWith(OV);
   BO->eraseFromParent();
   Cmp->eraseFromParent();
+  return true;
 }
 
 /// Try to combine the compare into a call to the llvm.uadd.with.overflow
 /// intrinsic. Return true if any changes were made.
 static bool combineToUAddWithOverflow(CmpInst *Cmp, const TargetLowering &TLI,
-                                      const DataLayout &DL) {
+                                      const DataLayout &DL, bool &ModifiedDT) {
   Value *A, *B;
   BinaryOperator *Add;
   if (!match(Cmp, m_UAddWithOverflow(m_Value(A), m_Value(B), m_BinOp(Add))))
@@ -1208,9 +1223,11 @@ static bool combineToUAddWithOverflow(Cm
     assert(*Add->user_begin() == Cmp && "expected!");
 #endif
 
-  Instruction *InPt = Add->hasOneUse() ? cast<Instruction>(Cmp)
-                                       : cast<Instruction>(Add);
-  replaceMathCmpWithIntrinsic(Add, Cmp, InPt, Intrinsic::uadd_with_overflow);
+  if (!replaceMathCmpWithIntrinsic(Add, Cmp, Intrinsic::uadd_with_overflow))
+    return false;
+
+  // Reset callers - do not crash by iterating over a dead instruction.
+  ModifiedDT = true;
   return true;
 }
 
@@ -1263,15 +1280,9 @@ static bool combineToUSubWithOverflow(Cm
                                 TLI.getValueType(DL, Sub->getType())))
     return false;
 
-  // Pattern matched and profitability checked. Check dominance to determine the
-  // insertion point for an intrinsic that replaces the subtract and compare.
-  DominatorTree DT(*Sub->getFunction());
-  bool SubDominates = DT.dominates(Sub, Cmp);
-  if (!SubDominates && !DT.dominates(Cmp, Sub))
-    return false;
-  Instruction *InPt = SubDominates ? cast<Instruction>(Sub)
-                                   : cast<Instruction>(Cmp);
-  replaceMathCmpWithIntrinsic(Sub, Cmp, InPt, Intrinsic::usub_with_overflow);
+  if (!replaceMathCmpWithIntrinsic(Sub, Cmp, Intrinsic::usub_with_overflow))
+    return false;
+
   // Reset callers - do not crash by iterating over a dead instruction.
   ModifiedDT = true;
   return true;
@@ -1348,7 +1359,7 @@ static bool optimizeCmp(CmpInst *Cmp, co
   if (sinkCmpExpression(Cmp, TLI))
     return true;
 
-  if (combineToUAddWithOverflow(Cmp, TLI, DL))
+  if (combineToUAddWithOverflow(Cmp, TLI, DL, ModifiedDT))
     return true;
 
   if (combineToUSubWithOverflow(Cmp, TLI, DL, ModifiedDT))