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

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In general looks correct to me. You definitely should check the branch itself - not the condition.
In your case (divergent loop-exit) branch itself is divergent because of the control dependency.



================
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.
----------------
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.


================
Comment at: include/llvm/Analysis/DivergenceAnalysis.h:46
+  //
+  // Even if this function returns true, V may still be divergent when used
+  // in a different basic block.
----------------
Same as above


================
Comment at: test/Transforms/StructurizeCFG/uniform-regions.ll:34
+  %v = load i32, i32 addrspace(2)* %gep, align 4
+  %cc2 = icmp eq i32 %v, 0
+  br i1 %cc2, label %end.loop, label %for.end
----------------
This is misleading. I can guess that using %v in the comparison means that branch condition is divergent.
Nevertheless, I think that it's more clear if you explicitly use the divergent definition for %v.
like:

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


https://reviews.llvm.org/D40546