[llvm] r321653 - [BasicBlockUtils] Check for unreachable preds before updating LI in UpdateAnalysisInformation

[Volkan Keles via llvm-commits]

+llvm-commits

> On Jan 2, 2018, at 8:25 AM, Anna Thomas via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: annat
> Date: Tue Jan  2 08:25:50 2018
> New Revision: 321653
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=321653&view=rev
> Log:
> [BasicBlockUtils] Check for unreachable preds before updating LI in UpdateAnalysisInformation
> 
> Summary:
> We are incorrectly updating the LI when loop-simplify generates
> dedicated exit blocks for a loop. The issue is that there's an implicit
> assumption that the Preds passed into UpdateAnalysisInformation are
> reachable. However, this is not true and breaks LI by incorrectly
> updating the header of a loop.
> 
> One such case is when we generate dedicated exits when the exit block is
> a landing pad (through SplitLandingPadPredecessors). There maybe other
> cases as well, since we do not guarantee that Preds passed in are
> reachable basic blocks.
> 
> The added test case shows how loop-simplify breaks LI for the outer loop (and DT in turn)
> after we try to generate the LoopSimplifyForm.
> 
> Reviewers: davide, chandlerc, sanjoy
> 
> Reviewed By: davide
> 
> Subscribers: llvm-commits
> 
> Differential Revision: https://reviews.llvm.org/D41519
> 
> Modified:
>    llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
>    llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll
> 
> Modified: llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp?rev=321653&r1=321652&r2=321653&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp Tue Jan  2 08:25:50 2018
> @@ -323,6 +323,12 @@ static void UpdateAnalysisInformation(Ba
>   bool IsLoopEntry = !!L;
>   bool SplitMakesNewLoopHeader = false;
>   for (BasicBlock *Pred : Preds) {
> +    // Preds that are not reachable from entry should not be used to identify if
> +    // OldBB is a loop entry or if SplitMakesNewLoopHeader. Unreachable blocks
> +    // are not within any loops, so we incorrectly mark SplitMakesNewLoopHeader
> +    // as true and make the NewBB the header of some loop. This breaks LI.
> +    if (!DT->isReachableFromEntry(Pred))

Hi Anna,

This change breaks our internal bots because DT might be nullptr as it is optional. Is there another way to check this?  

Volkan

> +      continue;
>     // If we need to preserve LCSSA, determine if any of the preds is a loop
>     // exit.
>     if (PreserveLCSSA)
> 
> Modified: llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll?rev=321653&r1=321652&r2=321653&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll (original)
> +++ llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll Tue Jan  2 08:25:50 2018
> @@ -18,3 +18,52 @@ while.body115:
> foo:
>   br label %while.body115
> }
> +
> +; When loopsimplify generates dedicated exit block for blocks that are landing
> +; pads (i.e. innerLoopExit in this test), we should not get confused with the
> +; unreachable pred (unreachableB) to innerLoopExit.
> +define align 8 void @baz(i32 %trip) personality i32* ()* @wobble {
> +entry:
> +  br label %outerHeader
> +
> +outerHeader:
> +  invoke void @foo() 
> +          to label %innerPreheader unwind label %innerLoopExit
> +
> +innerPreheader:
> +  br label %innerH
> +
> +innerH:
> +  %tmp50 = invoke i8 * undef()
> +          to label %innerLatch unwind label %innerLoopExit
> +
> +innerLatch:
> +  %cmp = icmp slt i32 %trip, 42
> +  br i1 %cmp, label %innerH, label %retblock
> +
> +unreachableB:                                             ; No predecessors!
> +  %tmp62 = invoke i8 * undef()
> +          to label %retblock unwind label %innerLoopExit
> +
> +; undedicated exit block (preds from inner and outer loop)
> +; Also has unreachableB as pred.
> +innerLoopExit:
> +  %tmp65 = landingpad { i8*, i32 }
> +          cleanup
> +  invoke void @foo() 
> +          to label %outerHeader unwind label %unwindblock
> +
> +unwindblock:
> +  %tmp67 = landingpad { i8*, i32 }
> +          cleanup
> +  ret void
> +
> +retblock:
> +  ret void
> +}
> +
> +; Function Attrs: nounwind
> +declare i32* @wobble()
> +
> +; Function Attrs: uwtable
> +declare void @foo()
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits