[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:945
+/// Note that this is not getUnderlyingObject(), since that looks through
+/// operations like ptrmask which might modify the resource part.
+static Value *rsrcPartRoot(Value *V) {
----------------
Document why this traversal order is enough


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1224
+  if (isa<LoadInst>(I))
+    // TODO: Do we need to do something about atomic loads?
+    IID = Intrinsic::amdgcn_raw_ptr_buffer_load;
----------------
@nhaehnle Upstream atomic_load from graphics?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1280
+    case AtomicRMWInst::UDecWrap:
+      report_fatal_error("wrapping increment/decrement not supported for "
+                         "buffer resources and should've ben expanded away");
----------------
We might have instructions for this but no intrinsics?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1760
+        if (auto *C = dyn_cast<Constant>(U.get()))
+          HasFatPointers |= isBufferFatPtrConst(C);
+    }
----------------
Give isBufferFatPtrConst a cache but make it fully recursive


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1824
+    SmallVector<ReturnInst *> Returns;
+    CloneFunctionInto(NewF, F, CloneMap, CloneFunctionChangeType::GlobalChanges,
+                      Returns, "", nullptr, &StructTM, &Materializer);
----------------
What about making a new function and moving the basic blocks? (Example needed)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158463