[llvm] r246244 - Constant propagation after hitting assume(cmp) bugfix
Piotr Padlewski via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 23:36:17 PDT 2015
Hi Steven,
I can't revert it by myself right now, so please revert this 2 commits for
me. I will be very helpful if You will send me some some source, on which
it hangs out.
Best
Piotr
On Thu, Aug 27, 2015 at 10:41 PM, Steven Wu <stevenwu at apple.com> wrote:
> 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=
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150827/7ae9e832/attachment.html>
More information about the llvm-commits
mailing list