[llvm] [WIP][AMDGPU] combine uniform AMDGPU lane Intrinsics (PR #116953)

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 06:16:30 PST 2024


jayfoad wrote:

> > ```
> >   if (ballot_all(x)) {
> >     speculatable_use(x)
> >      ...
> >   }
> > ```
> > Which may then later be incorrectly hoisted:
> > ```
> >   speculatable_use(x) // x is not actually uniform anymore
> >   if (ballot_all(x)) { ... }
> > ```
> > There needs to be something tying the uniformity to the use position
>
> Actually there is more going on here than just a lack of `convergent`. `ballot_all(x)` is a uniform condition. It will allow convergent operations to be hoisted over it. So if `speculatable_use(x)` really does depend on `x` being uniform, then it `convergent` is not enough to stop it. But should this use have `speculatable` in the first place? By explicitly putting the `speculatable` attribute, the user is saying that the compiler is free to move hoist this use wherever other conditions are met. Perhaps they shouldn't be using `speculatable` in the absence of nuance about uniformity?

I still don't see any problem here. I would assume that any speculatable use does not have side effects, so really it just computes some result, e.g.:
```
  if (ballot_all(x)) {
    y = speculatable_use(x) // convergent
    use(y)
  }
```
After hoisting:
```
  y = speculatable_use(x) // convergent
  if (ballot_all(x)) {
    use(y)
  }
```
So x may not be uniform when we call speculatable_use, but we only use y if x was uniform. This seems fine, even if speculatable_use returns poison when its argument is not uniform. (As long as it doesn't trigger undefined behavior...)

https://github.com/llvm/llvm-project/pull/116953


More information about the llvm-commits mailing list