[PATCH] D132055: [LoopInterchange][PR57148] Ensure LCSSA form after loop interchnange

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

In D132055#3734791 <https://reviews.llvm.org/D132055#3734791>, @uabelho wrote:

> I have no idea if this is the right fix, but I've verified that it solves the problem that I reported in pr 57148.
> Thanks!
>
> However, that was a reduced version of the problem, and if I try the original non-reduced version (unfortunately for an out-of-tree target) I get another failure with this patch.
> So it might be a step in the right direction, but there are more problems lurking.
>
> A reduced version of the new problem is
>
>   opt -passes="loop-interchange" bbi-72571_2.ll -o /dev/null
>
> which results in
>
>   Instruction does not dominate all uses!
>     %i.166.lcssa = phi i16 [ %3, %vector.body85.split ]
>     %arrayidx60 = getelementptr inbounds [2 x [4 x i32]], ptr @c, i16 0, i16 %i.166.lcssa, i16 %j.165
>   LLVM ERROR: Broken module found, compilation aborted!
>
> F24200925: bbi-72571_2.ll <https://reviews.llvm.org/F24200925>

The problem in `bbi-72751_2.ll` is different from the original `bbi-72751.ll`. The problem and solution is described as follows.

If we have a loop nest like this:

  for.outermost.header:
    %phi.outermost = ...
  
  for.middle.header:
    %phi.middle= ...
    use of %phi.outermost
  
  for.innermost:
    ....

If we interchange the outermost and the middle loop, it would become:

  // the new outermost loop header
  for.middle.header:
    %phi.middle= ...
    use of %phi.outermost
  
  // the new middle loop header
  for.outermost.header:
    %phi.outermost = ...
  
  for.innermost:
    ....

And we'll end up with the problem that we have use of `%phi.outermost` before its def.

The solution is that we split the inner loop header to two basic blocks where one contains only phi instructions. We already did this splitting for the innermost loop header which usually is also the innermost loop latch (so it is a single BB that contains phis and other instructions). With this patch we do the splitting for non-innermost loops as well.

For the case above this is how we do the splitting after this patch:

  for.outermost.header:
    %phi.outermost = ...
  
  for.middle.header:
    %phi.middle= ...
  
  for.middle.header.split:
    use of %phi.outermost
  
  for.innermost:
    ....

And now we can correctly interchange them:

  // the new outermost loop header
  for.middle.header:
    %phi.middle= ...
  
  // the new middle loop header
  for.outermost.header:
    %phi.outermost = ...
  
  for.middle.header.split:
    use of %phi.outermost
  
  for.innermost:
    ....

`bbi-72571_2.ll` is added in `llvm/test/Transforms/LoopInterchange/pr57148.ll` as another test case. Another change in test cases is `llvm/test/Transforms/LoopInterchange/pr43176-move-to-new-latch.ll` but it is just simple and better clean up of BBs, there is no functional change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132055/new/

https://reviews.llvm.org/D132055