[PATCH] D158463: [AMDGPU] Add IR-level pass to rewrite away address space 7

[Krzysztof Drewniak via Phabricator via llvm-commits]

krzysz00 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1175
+  if (IsVolatile)
+    Intr->setMetadata("amdgpu.volatile", MDNode::get(Ctx, {}));
+}
----------------
arsenm wrote:
> krzysz00 wrote:
> > arsenm wrote:
> > > What is this for? The safer thing to do would be record not volatile in metadata 
> > On the one hand, that'd make sense. On the other hand, are buffer loads and stores currently volatile (and so we'd have metadata marking them as non-volatile) or would volatile by default be a breaking semantics change?
> > 
> > The point of this was to not lose information.
> Assuming volatile by default is always conservatively correct. Since you technically can drop metadata, relying on it to preserve knowledge of volatileness is risky 
Could we, perhaps, use the high bits of the flags constant for volatile / non-volatile (ex. set bit 31 if this buffer op was IR-level volatile)?

Or would it make sense - since the `*.ptr.buffer.*` intrinsics aren't widely used yet - to go back and add alignment and volatility arguments to them?

Or is there some other approach I'm missing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158463/new/

https://reviews.llvm.org/D158463