llvm-reduce: Avoid removing convergent with convergence tokens #132946
Conversation
Check if the intrinsics are declared in the module as an overly conservative fix. Fixes #132695
| AttributeRemapper(Oracle &O, LLVMContext &C) : O(O), Context(C) {} | ||
| AttributeRemapper(Oracle &O, Module &M) : O(O), Context(M.getContext()) { | ||
|
|
||
| // Check if there are any convergence intrinsics used. We cannot remove the |
There was a problem hiding this comment.
Why not strip the control token along with the convergent attribute? The only place you can't strip a token is when it is the operand of the loop intrinsic, unless previous reductions make the loop intrinsic itself unused and it can be eliminated entirely.
There was a problem hiding this comment.
This reduction only is touching the attributes and should touch nothing else. It should only avoid doing something that will fail the verifier from the uses that require it. Deletion of the uses that may require the tokens is for another reduction pass
There was a problem hiding this comment.
Perhaps the last part of my comment about uses of the loop intrinsic was a distraction. My point is that the convergence control token can be viewed as an improvement over the convergent intrinsic, to the point that the intrinsic will become redundant if tokens are used everywhere. The token does not itself participate in control flow or data flow. In that sense, deleting the token on a convergent call while removing the convergent attribute is a valid interpretation of what this pass is doing. Even if we made a separate pass that removes token operands, that pass must remove the convergent attribute to keep the IR valid. Then we might as well do both right here.
There was a problem hiding this comment.
The point of the reduction is not remove the attribute at any cost. Any change in the reproducer is more likely related to the side effect of hacking on the actual instructions, not the attribute removal. It's not written to do so, nor should it be. Attributes that impose structural restrictions on the IR are problematic. It is fine to just ignore it.
arsenm
deleted the
users/arsenm/llvm-reduce/avoid-removing-convergent-with-convergence-tokens
branch
Check if the intrinsics are declared in the module as an overly
conservative fix.
Fixes #132695