[llvm] r331266 - Fix the issue that ComputeValueKnownInPredecessors only handles the case when

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue May 1 09:25:23 PDT 2018


> 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 <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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180501/4b6d82f2/attachment.html>


More information about the llvm-commits mailing list