[PATCH] D17013: Add convergent-removing bits to FunctionAttrs pass.

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:973
@@ +972,3 @@
+      return false;
+  }
+
----------------
jlebar wrote:
> joker.eph wrote:
> > Suggestion: outline the loop body in a helper function `canRemoveConvergent(CallGraphNode *CGN)`
> > 
> > The loop becomes then: `if(!llvm::all_of(SCC, canRemoveConvergent)) return false;`
> > 
> > 
> I'm actually not wild about this because it's a bit misleading.  Consider
> 
>   void foo() { bar(); }
>   void bar() { __syncthreads(); foo(); }
> 
> canRemoveConvergent(foo) would return true, even though, because we cannot remove convergent from bar and foo and bar are in the same SCC, we cannot in fact remove convergent from foo.
To be clear: I was suggesting a pure "syntactic" change no functional change intended. I agree that the naming I proposed isn't well suited though (considering your example)


http://reviews.llvm.org/D17013