ssahasra wrote:
Thanks for the example! I think #133684 is a correct incremental step forward,
but maybe just going the whole way bottom-up is better. It is also correct to
remove the verifier check. It's not a well-formedness error or even a semantic
error to pass a convergence control token
llvmbot wrote:
@llvm/pr-subscribers-llvm-ir
Author: Nathan Gauër (Keenuts)
Changes
When a callee is marked as `convergent`, some targets like HLSL/SPIR-V add a
convergent token to the call.
This is valid if both functions are marked as `convergent`.
ADCE/BDCE and other DCE passes were al
ssahasra wrote:
To take this to its logical conclusion, when convergence tokens are in use, the
`convergent` attribute is redundant. All we need is a `noconvergent` attribute
for function declarations. A function definition is convergent iff the body
contains a call to the `entry` intrinsic, a
arsenm wrote:
Thinking we should remove the IR verification requirement. It's more of a lint
type check and violations would be UB
https://github.com/llvm/llvm-project/pull/134844
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.
Keenuts wrote:
miss-typed enter, sent the comment above too early.
There are multiple ways to solve this issue:
- fix the FunctionAttr to run DCE again once the `convergent` attribute is
removed to make sure those invalid cases don't happen.
- fix the FunctionAttr to not remove `convergent` i
Keenuts wrote:
FYI: there is this PR which I think will replace this one:
https://github.com/llvm/llvm-project/pull/134863
> I didn't understand the validity part. Why is the caller required to be
> convergent in order to add a token to a callsite?
Given this example:
```llvm
declare i32 @fo
ssahasra wrote:
> When a callee is marked as convergent, some targets like HLSL/SPIR-V add a
> convergent token to the call.
This is valid if both functions are marked as convergent.
I didn't understand the validity part. Why is the caller required to be
convergent in order to add a token to a
ssahasra wrote:
>From the spec for convergence control tokens:
https://llvm.org/docs/ConvergentOperations.html#inferring-non-convergence
> An optimizer may remove the convergent attribute on a function if it can
> prove that the function does not contain a call to
> `llvm.experimental.converge
arsenm wrote:
> > Turns out not really, I ran spec with this about 2 years ago and the only
> > non-noise change was a mild improvement
>
> Looking at the PR you linked, seems like there was still not a clear
> consensus on the default change no?
Yes, there were people who are wrong and need
Keenuts wrote:
> Turns out not really, I ran spec with this about 2 years ago and the only
> non-noise change was a mild improvement
Looking at the PR you linked, seems like there was still not a clear consensus
on the default change no? (And I'd assume consumers like llvm-translator won't
be
arsenm wrote:
> I suspect the long-term change to change the default IR to assume convergent
> will take some time as it will impact many subprojects.
Turns out not really, I ran spec with this about 2 years ago and the only
non-noise change was a mild improvement
> Would you be OK with me
Keenuts wrote:
> These should not have side effects.
I agree I'm stretching the "hasSideEffect" definition here.
> Whatever is removing the convergent needs to consider the uses (or more
> likely, just leave it alone)
I suspect the long-term change to change the default IR to assume convergen
https://github.com/arsenm requested changes to this pull request.
These should not have side effects. The problem you are experiencing is because
the design of the convergent attribute is broken. It imposes a structural
requirement on the IR, rather than relaxing an assumed restriction. Whateve
llvmbot wrote:
@llvm/pr-subscribers-backend-spir-v
Author: Nathan Gauër (Keenuts)
Changes
When a callee is marked as `convergent`, some targets like HLSL/SPIR-V add a
convergent token to the call.
This is valid if both functions are marked as `convergent`.
ADCE/BDCE and other DCE passes
https://github.com/Keenuts created
https://github.com/llvm/llvm-project/pull/134844
When a callee is marked as `convergent`, some targets like HLSL/SPIR-V add a
convergent token to the call.
This is valid if both functions are marked as `convergent`.
ADCE/BDCE and other DCE passes were allowed
15 matches
Mail list logo