[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:
> 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.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:298
+  }
+  bool IsUniqued = !isa<StructType>(Ty) || cast<StructType>(Ty)->isLiteral();
+  // Base case for ints, floats, opaque pointers, and so on, which don't
----------------
arsenm wrote:
> arsenm wrote:
> > dyn_cast? Also could make a predicate function and clarify what uniqued means 
> not done?
Moved to a dyn_cast and added a comment - I don't think we need to create a whole function for this boolean


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158463