sameerds added a comment. In D69498#2708990 <https://reviews.llvm.org/D69498#2708990>, @arsenm wrote:
> You're still saying the same thing. This needs to be defined generically. > Frontends don't *have* to to anything, they'll just get the assumed > convergent behavior by default. Either frontends could always add > noconvergent to avoid any possible optimization hit, or we could have a pass > add noconvergent depending on the target. I don't want to change the > interpretation of core attributes based on the target Like I said earlier, this not a reinterpretation base on target. I think we are not talking about the same "convergent" here. There is currently an attribute in LLVM with the spelling "c-o-n-v-e-r-g-e-n-t". It does not do what its name says. It has a prescriptive definition that says "thou shalt not add control dependences to this function". This is not what we actually need, because it fails in two ways: 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69498/new/ https://reviews.llvm.org/D69498 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
