[PATCH] D40546: StructurizeCFG: Test for branch divergence correctly

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: include/llvm/Analysis/DivergenceAnalysis.h:40
+  //
+  // Even if this function returns false, V may still be divergent when used
+  // in a different basic block.
----------------
nhaehnle wrote:
> alex-t wrote:
> > Comment is not necessary. By definition:
> > 
> > instruction produces uniform result if and only if all it's operands are uniform and all the branches it control depend of  are uniform.
> > 
> > This comment assume that the person who uses Divergence Analysis has no idea of what it is.
> Strongly disagree. This is the header/interface file, and the definition you cite appears nowhere in the code, so the comment is valuable. To elaborate:
> 
> > This comment assume that the person who uses Divergence Analysis has no idea of what it is.
> 
> Kind of, though I would rather say: the comment assumes that the person who uses DivergenceAnalysis has no **experience** with it, and that is entirely fair. Actually, I'd say the mere existence of this patch is evidence that the comment makes sense.
> 
> You see, this patch fixes a bug in StructurizeCFG, which only uses divergence analysis optionally, and may be used by targets where divergence is not an issue. So it is entirely reasonable that a person who has no experience at all with DivergenceAnalysis would work on StructurizeCFG in a way that happens to touch how it interacts with DivergenceAnalysis. This comment serves as a warning to someone like that.
This problem may occur in case the value that is defined uniformly in a loop is used outside the loop in another block that is control dependent of the divergent loop exit branch. The value itself it is still uniform.
But the value that is produced by the user is divergent because of the divergent control dependency. So DivergentAnalysis is correct. The problem is in misunderstanding of what is control dependencies in the context of divergence.

I'd give same example as in https://reviews.llvm.org/D40547

%tid = call i32 @llvm.amdgcn.workitem.id.x()

for.body:

%val = add i32 %val, 1                                               <== definition of %val is uniform
%cmp = icmp gt i64 %tid, %arg1
br i1 %cmp, label %for.end, label %for.body             <== loop exit condition is divergent
for.end:

%result = add i32 %val, %x     <== each thread will have different %val here.  **So the %result is divergent but %val is still uniform**

Anyway it is not a serious issue. I just want to explain what I meant.




Repository:
  rL LLVM

https://reviews.llvm.org/D40546