[PATCH] D54516: [AMDGPU] Do not mark llvm.amdgcn.set.inactive as IntrNoMem

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

In https://reviews.llvm.org/D54516#1298178, @cwabbott wrote:

> I believe the combination of Convergent + not Speculatable should mean that the compiler shouldn't hoist it to a non-control-equivalent block and shouldn't CSE it. In particular, IIRC it's not guaranteed that that a readnone function always return the same value when it's called with the same arguments, so it's not safe to CSE -- it just means that LLVM can move other things across it, since it doesn't modify *caller-visible* state. What pass is causing a problem? Maybe it's a bug in the pass?


`readnone` is literally readnone. Maybe you're mixing this up with `inaccessiblememonly`?

I'm also not convinced that this patch actually fixes the problem. If you have code like this:

  if (cc) {
    WWM code
    other stuff v1
  } else {
    identical WWM code
    other stuff v2
  }

then it is perfectly correct to transform this to

  WWM code
  if (cc) {
    other stuff v1
  } else
    other stuff v2
  }

based on LLVM IR semantics. That's why we have the ugly "pass-through inline assembly" hack in the Mesa frontends right now for ballot etc.

We really need the `convergent` that prevents movement across control flow in either direction.


Repository:
  rL LLVM

https://reviews.llvm.org/D54516