[llvm] r246244 - Constant propagation after hitting assume(cmp) bugfix

Steven Wu via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 27 22:41:27 PDT 2015


Hi Piotr

I suspect this commit or the previous one actually hangs the clang bootstrap/self host. Here is one of the bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/3200
The previous one is failing but the stage 2 build was finished. This build causes timeout. As far as I see, it is looping somewhere in GVN.

Moreover, your previous commit in clang (r246213 or r246214) seems to hang the LTO bootstrap. Here is the bot:
http://lab.llvm.org:8080/green/job/llvm-stage2-cmake-RgLTO/2607/
LTO linking llvm-cov is hanging in ScalarEvolution. Might be triggered by something in your code?

Would you please revert these changes? Let me know if you need any assistant to reproduce the hang.

Thanks

Steven
 

> On Aug 27, 2015, at 6:02 PM, Piotr Padlewski via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: prazek
> Date: Thu Aug 27 20:02:00 2015
> New Revision: 246244
> 
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D246244-26view-3Drev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=8G3E4p0QiSQGRFFW3utOydBELevWunZREK4rHoMS5CA&e= 
> Log:
> Constant propagation after hitting assume(cmp) bugfix
> 
> Last time code run into assertion `BBE.isSingleEdge()` in
> lib/IR/Dominators.cpp:200.
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D12170&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=JAhAKhMDX_dxaFIWIHg1u7VUQ9mr4Zq20vHOtPnifUw&e= 
> 
> Modified:
>    llvm/trunk/include/llvm/Transforms/Utils/Local.h
>    llvm/trunk/lib/IR/Dominators.cpp
>    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>    llvm/trunk/lib/Transforms/Utils/Local.cpp
>    llvm/trunk/test/Transforms/GVN/assume-equal.ll
> 
> Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_Transforms_Utils_Local.h-3Frev-3D246244-26r1-3D246243-26r2-3D246244-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=4Z8rBUwlgMkkSgwH7hQuXwsWc7NpY-mhwqV5D69O3a0&e= 
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Thu Aug 27 20:02:00 2015
> @@ -291,6 +291,10 @@ void combineMetadata(Instruction *K, con
> /// the given edge.  Returns the number of replacements made.
> unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
>                                   const BasicBlockEdge &Edge);
> +/// \brief Replace each use of 'From' with 'To' if that use is dominated by
> +/// the given BasicBlock. Returns the number of replacements made.
> +unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
> +                                  const BasicBlock *BB);
> } // End llvm namespace
> 
> #endif
> 
> Modified: llvm/trunk/lib/IR/Dominators.cpp
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_IR_Dominators.cpp-3Frev-3D246244-26r1-3D246243-26r2-3D246244-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=bcx_fsnn91sHHcxhLIuXCVoHzRkQIrCVPCT5rbP3NGY&e= 
> ==============================================================================
> --- llvm/trunk/lib/IR/Dominators.cpp (original)
> +++ llvm/trunk/lib/IR/Dominators.cpp Thu Aug 27 20:02:00 2015
> @@ -147,7 +147,8 @@ bool DominatorTree::dominates(const Basi
>   // Assert that we have a single edge. We could handle them by simply
>   // returning false, but since isSingleEdge is linear on the number of
>   // edges, the callers can normally handle them more efficiently.
> -  assert(BBE.isSingleEdge());
> +  assert(BBE.isSingleEdge() &&
> +         "This function is not efficient in handling multiple edges");
> 
>   // If the BB the edge ends in doesn't dominate the use BB, then the
>   // edge also doesn't.
> @@ -197,7 +198,8 @@ bool DominatorTree::dominates(const Basi
>   // Assert that we have a single edge. We could handle them by simply
>   // returning false, but since isSingleEdge is linear on the number of
>   // edges, the callers can normally handle them more efficiently.
> -  assert(BBE.isSingleEdge());
> +  assert(BBE.isSingleEdge() &&
> +         "This function is not efficient in handling multiple edges");
> 
>   Instruction *UserInst = cast<Instruction>(U.getUser());
>   // A PHI in the end of the edge is dominated by it.
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_GVN.cpp-3Frev-3D246244-26r1-3D246243-26r2-3D246244-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=h6iUmj_kydFoOFu30aeWCv14ebRFbhknvH1hSYsWTpw&e= 
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Aug 27 20:02:00 2015
> @@ -725,7 +725,8 @@ namespace {
>     bool splitCriticalEdges();
>     BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
>     bool replaceOperandsWithConsts(Instruction *I) const;
> -    bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root);
> +    bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root,
> +                           bool DominatesByEdge);
>     bool processFoldableCondBr(BranchInst *BI);
>     void addDeadBlock(BasicBlock *BB);
>     void assignValNumForDeadCode();
> @@ -1777,9 +1778,17 @@ bool GVN::processAssumeIntrinsic(Intrins
> 
>     // This property is only true in dominated successors, propagateEquality
>     // will check dominance for us.
> -    Changed |= propagateEquality(V, True, Edge);
> +    Changed |= propagateEquality(V, True, Edge, false);
>   }
> -
> +  // We can replace assume value with true, which covers cases like this:
> +  // call void @llvm.assume(i1 %cmp)
> +  // br i1 %cmp, label %bb1, label %bb2 ; will change %cmp to true
> +  ReplaceWithConstMap[V] = True;
> +
> +  // If one of *cmp *eq operand is const, adding it to map will cover this:
> +  // %cmp = fcmp oeq float 3.000000e+00, %0 ; const on lhs could happen
> +  // call void @llvm.assume(i1 %cmp)
> +  // ret float %0 ; will change it to ret float 3.000000e+00
>   if (auto *CmpI = dyn_cast<CmpInst>(V)) {
>     if (CmpI->getPredicate() == CmpInst::Predicate::ICMP_EQ ||
>         CmpI->getPredicate() == CmpInst::Predicate::FCMP_OEQ ||
> @@ -2088,8 +2097,9 @@ bool GVN::replaceOperandsWithConsts(Inst
> /// The given values are known to be equal in every block
> /// dominated by 'Root'.  Exploit this, for example by replacing 'LHS' with
> /// 'RHS' everywhere in the scope.  Returns whether a change was made.
> -bool GVN::propagateEquality(Value *LHS, Value *RHS,
> -                            const BasicBlockEdge &Root) {
> +/// If DominatesByEdge is false, then it means that it is dominated by Root.End.
> +bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root,
> +                            bool DominatesByEdge) {
>   SmallVector<std::pair<Value*, Value*>, 4> Worklist;
>   Worklist.push_back(std::make_pair(LHS, RHS));
>   bool Changed = false;
> @@ -2146,7 +2156,11 @@ bool GVN::propagateEquality(Value *LHS,
>     // LHS always has at least one use that is not dominated by Root, this will
>     // never do anything if LHS has only one use.
>     if (!LHS->hasOneUse()) {
> -      unsigned NumReplacements = replaceDominatedUsesWith(LHS, RHS, *DT, Root);
> +      unsigned NumReplacements =
> +          DominatesByEdge
> +              ? replaceDominatedUsesWith(LHS, RHS, *DT, Root)
> +              : replaceDominatedUsesWith(LHS, RHS, *DT, Root.getEnd());
> +
>       Changed |= NumReplacements > 0;
>       NumGVNEqProp += NumReplacements;
>     }
> @@ -2218,7 +2232,10 @@ bool GVN::propagateEquality(Value *LHS,
>         Value *NotCmp = findLeader(Root.getEnd(), Num);
>         if (NotCmp && isa<Instruction>(NotCmp)) {
>           unsigned NumReplacements =
> -            replaceDominatedUsesWith(NotCmp, NotVal, *DT, Root);
> +              DominatesByEdge
> +                  ? replaceDominatedUsesWith(NotCmp, NotVal, *DT, Root)
> +                  : replaceDominatedUsesWith(NotCmp, NotVal, *DT,
> +                                             Root.getEnd());
>           Changed |= NumReplacements > 0;
>           NumGVNEqProp += NumReplacements;
>         }
> @@ -2292,11 +2309,11 @@ bool GVN::processInstruction(Instruction
> 
>     Value *TrueVal = ConstantInt::getTrue(TrueSucc->getContext());
>     BasicBlockEdge TrueE(Parent, TrueSucc);
> -    Changed |= propagateEquality(BranchCond, TrueVal, TrueE);
> +    Changed |= propagateEquality(BranchCond, TrueVal, TrueE, true);
> 
>     Value *FalseVal = ConstantInt::getFalse(FalseSucc->getContext());
>     BasicBlockEdge FalseE(Parent, FalseSucc);
> -    Changed |= propagateEquality(BranchCond, FalseVal, FalseE);
> +    Changed |= propagateEquality(BranchCond, FalseVal, FalseE, true);
> 
>     return Changed;
>   }
> @@ -2318,7 +2335,7 @@ bool GVN::processInstruction(Instruction
>       // If there is only a single edge, propagate the case value into it.
>       if (SwitchEdges.lookup(Dst) == 1) {
>         BasicBlockEdge E(Parent, Dst);
> -        Changed |= propagateEquality(SwitchCond, i.getCaseValue(), E);
> +        Changed |= propagateEquality(SwitchCond, i.getCaseValue(), E, true);
>       }
>     }
>     return Changed;
> 
> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Utils_Local.cpp-3Frev-3D246244-26r1-3D246243-26r2-3D246244-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=VaY7U2u44R2tDHWpKSFynev5sAO2KXWdXowhG2UxQzc&e= 
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Thu Aug 27 20:02:00 2015
> @@ -1349,3 +1349,23 @@ unsigned llvm::replaceDominatedUsesWith(
>   }
>   return Count;
> }
> +
> +unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
> +                                        DominatorTree &DT,
> +                                        const BasicBlock *BB) {
> +  assert(From->getType() == To->getType());
> +
> +  unsigned Count = 0;
> +  for (Value::use_iterator UI = From->use_begin(), UE = From->use_end();
> +       UI != UE;) {
> +    Use &U = *UI++;
> +    auto *I = cast<Instruction>(U.getUser());
> +    if (DT.dominates(BB, I->getParent())) {
> +      U.set(To);
> +      DEBUG(dbgs() << "Replace dominated use of '" << From->getName() << "' as "
> +                   << *To << " in " << *U << "\n");
> +      ++Count;
> +    }
> +  }
> +  return Count;
> +}
> 
> Modified: llvm/trunk/test/Transforms/GVN/assume-equal.ll
> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Transforms_GVN_assume-2Dequal.ll-3Frev-3D246244-26r1-3D246243-26r2-3D246244-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=u5vPTRrvDy5AhQWOCPbbsiKzGay_3w0RWL4KceWmosY&e= 
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/assume-equal.ll (original)
> +++ llvm/trunk/test/Transforms/GVN/assume-equal.ll Thu Aug 27 20:02:00 2015
> @@ -99,8 +99,6 @@ entry:
> }
> 
> ; CHECK-LABEL: define float @_Z1if(float %p)
> -
> -
> define float @_Z1if(float %p) {
> entry:
>   %p.addr = alloca float, align 4
> @@ -114,6 +112,23 @@ entry:
>   ret float %0
> }
> 
> +; This test checks if constant propagation works for multiple node edges
> +; CHECK-LABEL: define i32 @_Z1ii(i32 %p)
> +define i32 @_Z1ii(i32 %p) {
> +entry:
> +  %cmp = icmp eq i32 %p, 42
> +  call void @llvm.assume(i1 %cmp)
> +  
> +  ; CHECK: br i1 true, label %bb2, label %bb2
> +  br i1 %cmp, label %bb2, label %bb2
> +bb2:
> +  ; CHECK: br i1 true, label %bb2, label %bb2
> +  br i1 %cmp, label %bb2, label %bb2
> +  
> +  ; CHECK: ret i32 42
> +  ret i32 %p
> +}
> +
> declare noalias i8* @_Znwm(i64)
> declare void @_ZN1AC1Ev(%struct.A*)
> declare void @llvm.assume(i1)
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=AyppgLQmqA6F6mGKjHYJQMsYFVSyBzNb052X2-XqxQo&m=OEVOCQ_n7QuyS9xAvppvBAc08nxqpaB-CBau66yj_oA&s=P5r-R6T3ZBFQxiuD77x-_EEgsnbkRkhpIJSvqcH9_zc&e= 



More information about the llvm-commits mailing list