[PATCH] D81806: [DivergenceAnalysis] mark join of divergent loop exits

[Simon Moll via Phabricator via llvm-commits]

simoll added a comment.

In D81806#2094960 <https://reviews.llvm.org/D81806#2094960>, @sameerds wrote:

> In D81806#2094869 <https://reviews.llvm.org/D81806#2094869>, @simoll wrote:
>
> > I am a bit surprised this is necessary. Actually, join divergence in `bb3` should come about as a result of `propagateLoopDivergence` for the `[bb1, bb2]` loop and not immediately because it is a divergent exit.
>
>
> I am not sure what you mean here. propagateLoopDivergence is calling propagateJoinDivergence ... so doesn't that count as a result of the former?
>
> > Also, a divergent loop exit does not automatically become join divergent, eg:
> > 
> >   bb1: 
> >      %m.uni = <uni>
> >      %n.uni = <uni>
> >      br <uni>, %bb2, %bb3
> >    
> >   bb2: [..]
> >      br <div>, %bb2, %bb3
> >    
> >   bb3:
> >      %x.uni = phi [%m.uni, %bb2], [%n.uni %bb1]  <-- divergent loop exit that is not join divergent
>
> But here, bb3 is not a join block for the bb2 loop, since there is only one path from the loop exit reaching bb3.


Exactly. With this patch `%x.uni` is marked as join divergent even though it is uniform: In its present form the patch solves one issue at the cost of precision.

The real culprit here is the `SDA::join_blocks` interface that does not distinguish divergent join blocks and loop exits. The SDA "knows" which is which internally, loses that information as it passes divergent blocks through `join_blocks` and the DA has to reverse-engineer that information. I have an unfinished patch here that fixes that and should resolve the bug addressed by this patch as well. The unfinished patch also factors the back-and-forth between the DA and SDA to propagate along nested loops completely into the SDA (`propagateLoopDiv` does not really belong in the DA).

Long story short: This is an ok fix (but please add a FIXME/TODO that hints to the issues with this approach) but we should rework the `join_blocks` interface in any case.



================
Comment at: llvm/lib/Analysis/DivergenceAnalysis.cpp:290
+  // disjoint-paths join at JoinBlock
   markBlockJoinDivergent(JoinBlock);
+
----------------
Add a FIXME here that clearly states that this is a quickfix. You could also add the following test (in comments since it will fail) to your test case to document the imprecision caused by this:

    ; CHECK: bb2:
    ; CHECK-NOT: DIVERGENT:       %Guard.bb2 = phi i1 [ true, %bb1 ], [ false, %bb0 ]

    define protected amdgpu_kernel void @test2(i1 %uni) {
    bb0:
      %tid.x = call i32 @llvm.amdgcn.workitem.id.x()
      %i5 = icmp eq i32 %tid.x, -1
      br i1 %uni, label %bb1, label %bb2

    bb1:                                              ; preds = %bb2, %bb0
      %lsr.iv = phi i32 [ 7, %bb0 ], [ %lsr.iv.next, %bb1 ]
      %lsr.iv.next = add nsw i32 %lsr.iv, -1
      br i1 %i5, label %bb2, label %bb1

    bb2:                                              ; preds = %bb2, %bb1
      %Guard.bb2 = phi i1 [ true, %bb1 ], [ false, %bb0 ]
      ret void
    }

    attributes #0 = { nounwind readnone speculatable }



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81806