[PATCH] D25075: [JumpThreading] Allow threading over loop headers when creating single block loops

[Philip Reames via llvm-commits]

reames added inline comments.


> JumpThreading.cpp:1460
> +  if (LoopHeaders.count(BB) &&
> +      std::find(PredBBs.begin(), PredBBs.end(), SuccBB) == PredBBs.end()) {
>      DEBUG(dbgs() << "  Not threading across loop header BB '" << BB->getName()

I'm more than a bit nervous about this check.  I'm trying to convince myself it's correct and am struggling.

> JumpThreading.cpp:1596
>  
> +  if (LoopHeaders.count(BB))
> +    // We threaded over a loop header. Recalculate loop headers now.

Some form of incremental update would be good.

> singleblock-loop.ll:7
> +; CHECK-LABEL: @f
> +; CHECK: while.body: ; preds = %while.cond.thread, %while.cond
> +define void @f() {

Please give more detailed checks.  It's really hard to tell what the outcome here is.

> singleblock-loop.ll:12
> +
> +while.cond:                                       ; preds = %while.body, %entry
> +  %i.0 = phi i32 [ 2, %entry ], [ %add.i.0, %while.body ]

Looking at the example, it seems like we actually have two sub-cases you want to handle:

- thread a branch through a header to a loop exit
- thread a branch through a header to itself (essentially removing the header from the loop)

The former is obviously correct and easy to reason about.  The second is the tricky case if we want to handle it generally while avoiding irreducible control flow.

I wonder if this would be easier to reason about if you broke apart each sub-case explicitly?

> singleblock-loop.ll:18
> +while.body:                                       ; preds = %while.cond
> +  %add = add nsw i32 %i.0, 1
> +  %call = call i32 @g()

Not related to this review, but we should be able to prove this is "3".  Is the actual case you're worried about this fragile?  If so, there are other simpler patches which might work.

Repository:
  rL LLVM

https://reviews.llvm.org/D25075