[PATCH] D69498: IR: Invert convergent attribute handling

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D69498#2709218 <https://reviews.llvm.org/D69498#2709218>, @sameerds wrote:

> 1. It is actually okay add control dependences to a convergent function as long as the new branches are uniform.
> 2. Some times we hack a transform to avoid doing things that cannot be described as "add new control dependence", and still talk about the function having the above named attribute.
>
> There is another important bug that obfuscates the whole discussion: most places that use "isConvergent()" should in fact first check if it really matters to the surrounding control flow. There is too much loaded onto that one attribute, without sufficient context. The definition of "noconvergent" that I am proposing starts out by first fixing the definition of convergent itself. This definition is independent of target, and only talks about the properties of the control flow that reach the callsite. To repeat, this does not reinterpret the IR in a target-defined way. Like I said, in the new definition, all function calls are convergent even on CPUs, so I don't see where the target comes in. If you still insist on talking about interpretation of core attributes, please start by deconstructing the definition that I propose so I can see what I am missing.

Yes, the current concept of convergent is broken. I think this whole recent burst of conversation has been too focused on the current convergent situation and this patch in particular. I think we need to conceptually rebase this into the world that D85603 <https://reviews.llvm.org/D85603> creates. The expected convergence behavior is not only a target property, but of the source language. The frontend ultimately has to express the intended semantics of these cross lane regions, and the convergence tokens gives this ability.

My point about not making this target dependent is we shouldn't be trying to shoehorn in TTI checks around convergent operations just to save frontends from the inconvenience of adding another attribute to function declarations. We can interprocedurally infer noconvergent like any other attribute most of the time. The convergent calls and their tokens should be interpreted the same even if the target CPU doesn't really have cross lane behavior.


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

https://reviews.llvm.org/D69498