From 00fcc74e502a66244dc416c3f12534aedb2612d6 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sun, 3 Feb 2019 13:48:03 +0000 Subject: [PATCH] [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. llvm-svn: 352988 --- llvm/lib/CodeGen/CodeGenPrepare.cpp | 74 +++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp index 07ea1d0..792e4a5 100644 --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp @@ -1147,14 +1147,16 @@ static bool OptimizeNoopCopyExpression(CastInst *CI, const TargetLowering &TLI, 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(CmpInst *CI) { // 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(CmpInst *CI) { /// 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(CI)) + if (TLI.useSoftFloat() && isa(Cmp)) return false; // Only insert a cmp in each block once. DenseMap 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(*UI); @@ -1229,6 +1229,7 @@ static bool SinkCmpExpression(CmpInst *CI, const TargetLowering *TLI) { // 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 *CI, const TargetLowering *TLI) { 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 *CI, const TargetLowering *TLI) { } // 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(Instruction *I, bool &ModifiedDT) { } if (CmpInst *CI = dyn_cast(I)) - if (!TLI || !TLI->hasMultipleConditionRegisters()) - return OptimizeCmpExpression(CI, TLI); + if (TLI && optimizeCmpExpression(CI, *TLI)) + return true; if (LoadInst *LI = dyn_cast(I)) { LI->setMetadata(LLVMContext::MD_invariant_group, nullptr); -- 2.7.4