[llvm] r352988 - [CGP] refactor optimizeCmpExpression (NFCI)

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 3 05:48:04 PST 2019


Author: spatel
Date: Sun Feb  3 05:48:03 2019
New Revision: 352988

URL: http://llvm.org/viewvc/llvm-project?rev=352988&view=rev
Log:
[CGP] refactor optimizeCmpExpression (NFCI)

This is not truly NFC because we are bailing out without
a TLI now. That should not be a real concern though because
there should be a TLI in any real-world scenario. 

That seems better than passing around a pointer and then 
checking it for null-ness all over the place.

The motivation is to fix what appears to be an unintended
restriction on the uaddo transform - 
hasMultipleConditionRegisters() shouldn't be reason to limit 
the transform.

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=352988&r1=352987&r2=352988&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Sun Feb  3 05:48:03 2019
@@ -1147,14 +1147,16 @@ static bool OptimizeNoopCopyExpression(C
   return SinkCast(CI);
 }
 
-/// Try to combine CI into a call to the llvm.uadd.with.overflow intrinsic if
-/// possible.
-///
-/// Return true if any changes were made.
-static bool CombineUAddWithOverflow(CmpInst *CI) {
+/// 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) {
+  // TODO: Why is this transform limited by this condition?
+  if (TLI.hasMultipleConditionRegisters())
+    return false;
+
   Value *A, *B;
   Instruction *AddI;
-  if (!match(CI,
+  if (!match(Cmp,
              m_UAddWithOverflow(m_Value(A), m_Value(B), m_Instruction(AddI))))
     return false;
 
@@ -1165,35 +1167,32 @@ static bool CombineUAddWithOverflow(CmpI
   // We don't want to move around uses of condition values this late, so we we
   // check if it is legal to create the call to the intrinsic in the basic
   // block containing the icmp:
-
-  if (AddI->getParent() != CI->getParent() && !AddI->hasOneUse())
+  if (AddI->getParent() != Cmp->getParent() && !AddI->hasOneUse())
     return false;
 
 #ifndef NDEBUG
   // Someday m_UAddWithOverflow may get smarter, but this is a safe assumption
   // for now:
   if (AddI->hasOneUse())
-    assert(*AddI->user_begin() == CI && "expected!");
+    assert(*AddI->user_begin() == Cmp && "expected!");
 #endif
 
-  Module *M = CI->getModule();
+  Module *M = Cmp->getModule();
   Function *F = Intrinsic::getDeclaration(M, Intrinsic::uadd_with_overflow, Ty);
-
-  auto *InsertPt = AddI->hasOneUse() ? CI : AddI;
-
-  DebugLoc Loc = CI->getDebugLoc();
-  auto *UAddWithOverflow =
-      CallInst::Create(F, {A, B}, "uadd.overflow", InsertPt);
+  Instruction *InsertPt = AddI->hasOneUse() ? Cmp : AddI;
+  DebugLoc Loc = Cmp->getDebugLoc();
+  Instruction *UAddWithOverflow = CallInst::Create(F, {A, B}, "uadd.overflow",
+                                                   InsertPt);
   UAddWithOverflow->setDebugLoc(Loc);
-  auto *UAdd = ExtractValueInst::Create(UAddWithOverflow, 0, "uadd", InsertPt);
+  Instruction *UAdd = ExtractValueInst::Create(UAddWithOverflow, 0, "uadd",
+                                               InsertPt);
   UAdd->setDebugLoc(Loc);
-  auto *Overflow =
-      ExtractValueInst::Create(UAddWithOverflow, 1, "overflow", InsertPt);
+  Instruction *Overflow = ExtractValueInst::Create(UAddWithOverflow, 1,
+                                                   "overflow", InsertPt);
   Overflow->setDebugLoc(Loc);
-
-  CI->replaceAllUsesWith(Overflow);
+  Cmp->replaceAllUsesWith(Overflow);
   AddI->replaceAllUsesWith(UAdd);
-  CI->eraseFromParent();
+  Cmp->eraseFromParent();
   AddI->eraseFromParent();
   return true;
 }
@@ -1204,18 +1203,19 @@ static bool CombineUAddWithOverflow(CmpI
 /// lose; some adjustment may be wanted there.
 ///
 /// Return true if any changes are made.
-static bool SinkCmpExpression(CmpInst *CI, const TargetLowering *TLI) {
-  BasicBlock *DefBB = CI->getParent();
+static bool sinkCmpExpression(CmpInst *Cmp, const TargetLowering &TLI) {
+  if (TLI.hasMultipleConditionRegisters())
+    return false;
 
   // Avoid sinking soft-FP comparisons, since this can move them into a loop.
-  if (TLI && TLI->useSoftFloat() && isa<FCmpInst>(CI))
+  if (TLI.useSoftFloat() && isa<FCmpInst>(Cmp))
     return false;
 
   // Only insert a cmp in each block once.
   DenseMap<BasicBlock*, CmpInst*> InsertedCmps;
 
   bool MadeChange = false;
-  for (Value::user_iterator UI = CI->user_begin(), E = CI->user_end();
+  for (Value::user_iterator UI = Cmp->user_begin(), E = Cmp->user_end();
        UI != E; ) {
     Use &TheUse = UI.getUse();
     Instruction *User = cast<Instruction>(*UI);
@@ -1229,6 +1229,7 @@ static bool SinkCmpExpression(CmpInst *C
 
     // Figure out which BB this cmp is used in.
     BasicBlock *UserBB = User->getParent();
+    BasicBlock *DefBB = Cmp->getParent();
 
     // If this user is in the same block as the cmp, don't change the cmp.
     if (UserBB == DefBB) continue;
@@ -1240,10 +1241,11 @@ static bool SinkCmpExpression(CmpInst *C
       BasicBlock::iterator InsertPt = UserBB->getFirstInsertionPt();
       assert(InsertPt != UserBB->end());
       InsertedCmp =
-          CmpInst::Create(CI->getOpcode(), CI->getPredicate(),
-                          CI->getOperand(0), CI->getOperand(1), "", &*InsertPt);
+          CmpInst::Create(Cmp->getOpcode(), Cmp->getPredicate(),
+                          Cmp->getOperand(0), Cmp->getOperand(1), "",
+                          &*InsertPt);
       // Propagate the debug info.
-      InsertedCmp->setDebugLoc(CI->getDebugLoc());
+      InsertedCmp->setDebugLoc(Cmp->getDebugLoc());
     }
 
     // Replace a use of the cmp with a use of the new cmp.
@@ -1253,19 +1255,19 @@ static bool SinkCmpExpression(CmpInst *C
   }
 
   // If we removed all uses, nuke the cmp.
-  if (CI->use_empty()) {
-    CI->eraseFromParent();
+  if (Cmp->use_empty()) {
+    Cmp->eraseFromParent();
     MadeChange = true;
   }
 
   return MadeChange;
 }
 
-static bool OptimizeCmpExpression(CmpInst *CI, const TargetLowering *TLI) {
-  if (SinkCmpExpression(CI, TLI))
+static bool optimizeCmpExpression(CmpInst *Cmp, const TargetLowering &TLI) {
+  if (sinkCmpExpression(Cmp, TLI))
     return true;
 
-  if (CombineUAddWithOverflow(CI))
+  if (combineToUAddWithOverflow(Cmp, TLI))
     return true;
 
   return false;
@@ -6712,8 +6714,8 @@ bool CodeGenPrepare::optimizeInst(Instru
   }
 
   if (CmpInst *CI = dyn_cast<CmpInst>(I))
-    if (!TLI || !TLI->hasMultipleConditionRegisters())
-      return OptimizeCmpExpression(CI, TLI);
+    if (TLI && optimizeCmpExpression(CI, *TLI))
+      return true;
 
   if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
     LI->setMetadata(LLVMContext::MD_invariant_group, nullptr);




More information about the llvm-commits mailing list