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

[Krzysztof Drewniak via Phabricator via llvm-commits]

krzysz00 marked an inline comment as done.
krzysz00 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:240-241
+protected:
+  virtual Type *remapScalar(PointerType *PT) = 0;
+  virtual Type *remapVector(VectorType *VT) = 0;
+
----------------
arsenm wrote:
> What's the point in having these be separate?
> 
> Also not sure why these are virtual 
I had them separate because, in the struct case, remapVector(T) isn't <NxremapScalar(T)>.

And they're virtual because we have a base class method that'll call these methods and we want it to call the right ones depending on which subclass we're working with.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:283
+    Type *Ty, SmallPtrSetImpl<StructType *> &Seen) {
+  Type **Entry = &Map[Ty];
+  if (*Entry)
----------------
arsenm wrote:
> Type *&
> 
> Could you also move all the core logic to a separate function, such that every return doesn't need to consider the assignment to the map?
`Type *&` starts throwing compile errors.

Also, I'm basing this off existing code, and I suspect the whole set-entry-on-return setup has to do with updating everything correctly when there's recursion (self-referential structs and the like).


================
Comment at: llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-calls.ll:79
+  ret void
+}
----------------
arsenm wrote:
> Test some external declarations, and some cases with non-default linkage and attributes that should be preserved 
I think I got those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158463