[llvm] r331266 - Fix the issue that ComputeValueKnownInPredecessors only handles the case when
Wei Mi via llvm-commits
llvm-commits at lists.llvm.org
Tue May 1 09:44:27 PDT 2018
Sorry about the problem. Fixed in r331280.
Wei.
On Tue, May 1, 2018 at 9:25 AM, Adrian Prantl <aprantl at apple.com> wrote:
> On May 1, 2018, at 7:47 AM, Wei Mi via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>
> Author: wmi
> Date: Tue May 1 07:47:24 2018
> New Revision: 331266
>
> URL: http://llvm.org/viewvc/llvm-project?rev=331266&view=rev
> Log:
> Fix the issue that ComputeValueKnownInPredecessors only handles the case
> when
> phi is on lhs of a comparison op.
>
> For the following testcase,
> L1:
>
> %t0 = add i32 %m, 7
> %t3 = icmp eq i32* %t2, null
> br i1 %t3, label %L3, label %L2
>
> L2:
>
> %t4 = load i32, i32* %t2, align 4
> br label %L3
>
> L3:
>
> %t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]
> %t6 = icmp eq i32 %t0, %t5
> br i1 %t6, label %L4, label %L5
>
> We know if we go through the path L1 --> L3, %t6 should always be true.
> However
> currently, if the rhs of the eq comparison is phi, JumpThreading fails to
> evaluate %t6 to true. And we know that Instcombine cannot guarantee always
> canonicalizing phi to the left hand side of the comparison operation
> according
> to the operand priority comparison mechanism in instcombine. The patch
> handles
> the case when rhs of the comparison op is a phi.
>
> Differential Revision: https://reviews.llvm.org/D46275
>
>
> Modified:
> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> llvm/trunk/test/Other/pr32085.ll
> llvm/trunk/test/Transforms/JumpThreading/phi-known.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=331266&r1=331265&r2=331266&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue May 1 07:47:24
> 2018
> @@ -752,6 +752,8 @@ bool JumpThreadingPass::ComputeValueKnow
> CmpInst::Predicate Pred = Cmp->getPredicate();
>
> PHINode *PN = dyn_cast<PHINode>(CmpLHS);
> + if (!PN)
> + PN = dyn_cast<PHINode>(CmpRHS);
> if (PN && PN->getParent() == BB) {
> const DataLayout &DL = PN->getModule()->getDataLayout();
> // We can do this simplification if any comparisons fold to true or
> false.
> @@ -762,14 +764,24 @@ bool JumpThreadingPass::ComputeValueKnow
> LVI->enableDT();
> for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
> BasicBlock *PredBB = PN->getIncomingBlock(i);
> - Value *LHS = PN->getIncomingValue(i);
> - Value *RHS = CmpRHS->DoPHITranslation(BB, PredBB);
> -
> + Value *LHS, *RHS;
> + if (PN == CmpLHS) {
> + LHS = PN->getIncomingValue(i);
> + RHS = CmpRHS->DoPHITranslation(BB, PredBB);
> + } else {
> + LHS = CmpLHS->DoPHITranslation(BB, PredBB);
> + RHS = PN->getIncomingValue(i);
> + }
> Value *Res = SimplifyCmpInst(Pred, LHS, RHS, {DL});
> if (!Res) {
> if (!isa<Constant>(RHS))
> continue;
>
> + // getPredicateOnEdge call will make no sense if LHS is defined
> in BB.
> + auto LHSInst = dyn_cast<Instruction>(LHS);
> + if (LHSInst && LHSInst->getParent() == BB)
> + continue;
> +
> LazyValueInfo::Tristate
> ResT = LVI->getPredicateOnEdge(Pred, LHS,
> cast<Constant>(RHS), PredBB, BB,
>
> Modified: llvm/trunk/test/Other/pr32085.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/pr32085.ll?rev=331266&r1=331265&r2=331266&view=diff
> ==============================================================================
> --- llvm/trunk/test/Other/pr32085.ll (original)
> +++ llvm/trunk/test/Other/pr32085.ll Tue May 1 07:47:24 2018
> @@ -1,6 +1,7 @@
> ; RUN: opt -S -O1 < %s -o %t1.ll
> ;; Show that there's no difference after running another simplify CFG
> -; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll
> +; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll
> +; RUN: sed -i 's/; preds = .*//g' %t1.ll %t2.ll
>
>
> It looks like this doesn't play well with BSD sed
>
> http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/48381/consoleFull#17914833858254eaf0-7326-4999-85b0-388101f2d404
>
> Also, I would be surprised if this test worked on Windows?
>
> -- adrian
>
> ; RUN: diff %t1.ll %t2.ll
>
> ; Test from LoopSink pass, leaves some single-entry single-exit basic
> blocks.
>
> Modified: llvm/trunk/test/Transforms/JumpThreading/phi-known.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/phi-known.ll?rev=331266&r1=331265&r2=331266&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/JumpThreading/phi-known.ll (original)
> +++ llvm/trunk/test/Transforms/JumpThreading/phi-known.ll Tue May 1
> 07:47:24 2018
> @@ -64,3 +64,41 @@ exit:
> ret void
> }
>
> +; The eq predicate is always true if we go through the path from
> +; L1 to L3, no matter the phi result %t5 is on the lhs or rhs of
> +; the predicate.
> +declare void @goo()
> +declare void @hoo()
> +
> +define void @test3(i32 %m, i32** %t1) {
> +L1:
> + %t0 = add i32 %m, 7
> + %t2 = load i32*, i32** %t1, align 8
> +; CHECK-LABEL: @test3
> +; CHECK: %t3 = icmp eq i32* %t2, null
> +; CHECK: br i1 %t3, label %[[LABEL2:.*]], label %[[LABEL1:.*]]
> +
> + %t3 = icmp eq i32* %t2, null
> + br i1 %t3, label %L3, label %L2
> +
> +; CHECK: [[LABEL1]]:
> +; CHECK-NEXT: %t4 = load i32, i32* %t2, align 4
> +L2:
> + %t4 = load i32, i32* %t2, align 4
> + br label %L3
> +
> +L3:
> + %t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]
> + %t6 = icmp eq i32 %t0, %t5
> + br i1 %t6, label %L4, label %L5
> +
> +; CHECK: [[LABEL2]]:
> +; CHECK-NEXT: call void @goo()
> +L4:
> + call void @goo()
> + ret void
> +
> +L5:
> + call void @hoo()
> + ret void
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list