arsenm added inline comments.
================
Comment at: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu:62-64
+// OPT-NOT: alloca
+// OPT-NOT: ptrtoint
+// OPT-NOT: inttoptr
----------------
Positive checks are preferable (here and through the rest of the file)
================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:223
+static bool isNoopPtrIntCastPair(const Operator *I2P, const DataLayout *DL,
+ const TargetTransformInfo *TTI) {
----------------
Can you add a comment elaborating on why this is necessary? This is special
casing this to paper over the missing no-op pointer bitcast
================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:237-239
+ TTI->isNoopAddrSpaceCast(
+ P2I->getOperand(0)->getType()->getPointerAddressSpace(),
+ I2P->getType()->getPointerAddressSpace());
----------------
Can you also elaborate on why the isNoopAddrSpaceCast check is needed? I'm not
100% sure it is in all contexts, so want to be sure to document that
================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:245
// getelementptr operators.
-static bool isAddressExpression(const Value &V) {
+static bool isAddressExpression(const Value &V, const DataLayout *DL,
+ const TargetTransformInfo *TTI) {
----------------
data layout should be const reference
================
Comment at: llvm/test/Transforms/InferAddressSpaces/noop-ptrint-pair.ll:21
+; CHECK-NEXT: ret void
+define void @non_noop_ptrint_pair(i32 addrspace(3)* %x.coerce) {
+ %1 = ptrtoint i32 addrspace(3)* %x.coerce to i64
----------------
Can you also add one where the cast would be a no-op, but the integer size is
different? e.g. 0 to 1 and 1 to 0 where the intermediate size is 32-bits or 128
bits
================
Comment at: llvm/test/Transforms/InferAddressSpaces/noop-ptrint-pair.ll:45
+ ret void
+}
----------------
Can you add some cases where the pointer value isn't used for memory? Like pure
pointer arithmetic
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81938/new/
https://reviews.llvm.org/D81938
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
