[PATCH] D151987: TTI: Add function to hasBranchDivergence

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:403
+  /// If \p F is passed, provides a context function. This can be used to report
+  /// all values can be assumed uniform in a particular function.
+  bool hasBranchDivergence(const Function *F = nullptr) const;
----------------
Nit: the comment sounds ambiguous.

Is that "can be used to report all values *that* can be assumed [to be] uniform"?
Or "can be used to report that all values can be assumed [to be] uniform"?

Considering that we're returning a bool, I assume it's the latter.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:404
+  /// all values can be assumed uniform in a particular function.
+  bool hasBranchDivergence(const Function *F = nullptr) const;
 
----------------
Does it mean that with nullptr argument the function reports whether the target has a concept of divergence, but if passed an actual function, ti would report whether the function has divergent IR?

I'd argue that for such different purposes we should have separate function calls.
- `bool hasBranchDivergence()` would report whether target cares about divergence.
-  `bool isAlwaysUniform(const Function *F)` would report whether a given function is divergent.



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

https://reviews.llvm.org/D151987