[PATCH] D158463: [AMDGPU] Add IR-level pass to rewrite away address space 7
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 30 03:47:53 PDT 2023
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:24
+// semantics. This allows the frontend to reason about such buffers (which are
+// often encounterd in the context of SPIR-V kernels).
+//
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:37
+// encountering cases where this is required, such as by inserting these values
+// into aggretates ormoving them to memory.
+//
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:678
+ NewArg = CE->getIntegerCast(NewArg, OffTy, /*IsSigned=*/true);
+ if (Multiple.isPowerOf2())
+ NewArg = ConstantExpr::getShl(
----------------
Braces
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:684
+ /*hasNUW=*/InBounds, /*HasNSW=*/InBounds);
+ else
+ NewArg =
----------------
Braces
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:688
+ /*hasNUW=*/InBounds, /*hasNSW=*/InBounds);
+ if (OffAccum)
+ OffAccum = ConstantExpr::getAdd(OffAccum, NewArg, /*hasNUW=*/InBounds,
----------------
Braces
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1009
+
+ std::optional<Value *> MaybeRsrc = std::nullopt;
+ auto MaybeFoundRsrc = FoundRsrcs.find(I);
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1018
+ getPossibleRsrcRoots(I, Roots, Seen);
+ LLVM_DEBUG(llvm::dbgs() << "Processing conditional: " << *I << "\n");
+#ifndef NDEBUG
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1021
+ for (Value *V : Roots)
+ LLVM_DEBUG(llvm::dbgs() << "Root: " << *V << "\n");
+ for (Value *V : Seen)
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1023
+ for (Value *V : Seen)
+ LLVM_DEBUG(llvm::dbgs() << "Seen: " << *V << "\n");
+#endif
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1135
+ } else {
+ delete OffDbg;
+ }
----------------
Don't think you're supposed to use a raw delete on any llvm value? Is there an erase method of some kind?
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp:1175
+ if (IsVolatile)
+ Intr->setMetadata("amdgpu.volatile", MDNode::get(Ctx, {}));
+}
----------------
What is this for? The safer thing to do would be record not volatile in metadata
================
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:
> dyn_cast? Also could make a predicate function and clarify what uniqued means
not done?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158463/new/
https://reviews.llvm.org/D158463
More information about the llvm-commits
mailing list