[PATCH] D69498: IR: Invert convergent attribute handling

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added a comment.

In D69498#2706524 <https://reviews.llvm.org/D69498#2706524>, @arsenm wrote:

> In D69498#2705441 <https://reviews.llvm.org/D69498#2705441>, @sameerds wrote:
>
>> I realize now that what @foad says above puts the idea in a clearer context. Essentially, any check for isConvergent() isn't happening in a vacuum. It is relevant only in the presence of divergent control flow, which in turn is relevant only when the target has divergence. Any standalone check for isConvergent() is merely making a pessimistic assumption that all the control flow incident on it is divergent. This has two consequences:
>>
>> 1. The meaning of the `convergent` attribute has always been target-dependent.
>> 2. Dependence on TTI is not a real cost at all. We may eventually update every use of isConvergent() to depend on a check for divergence. The check for TTI is only the first step towards that.
>
> The core IR semantics must *not* depend on target interpretation. convergent has never been target dependent, and was defined in an abstract way. Only certain targets will really care, but we cannot directly define it to mean what we want it to mean for particular targets

My bad. I should have said "the implementation of convergent" is target dependent. But even that is not precise enough. It is more correct to say that the convergent attribute can be implemented inside LLVM by depending on TTI so that it only impacts targets that have divergence. This dependence is not new; it's merely missing because the current uses of convergent pessimistically assume divergent control flow on targets.

The important point is that frontends have to do nothing special for targets that don't have divergence, because the concepts of convergence and divergence have a trivial effect on optimizations. I had already observed in a previous comment that this new definition does not amount to reinterpreting the same program differently on different targets. It is perfectly okay to say that all calls are convergent on a CPU, because all branches are trivially uniform on the CPU (this part is inspired by @foad's observation), and hence the default convergent nature of calls has no effect on optimization for CPUs. If we were to rename `isConvergent()` to `hasConvergenceConstraints()` then we can see that it trivially returns `false` for any target that has no divergence.

Note that my proposed definition for `noconvergent` makes no mention of the target. The first sentence starts with "some targets", but that's just for a helpful context. The concept of convergent calls is defined purely in terms of divergent control flow. This one sentence very nicely ties everything up:

> In the presence of divergent control flow, such optimizations conservatively treat every call/invoke instruction as convergent by default.


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

https://reviews.llvm.org/D69498