[PATCH] D81938: [InferAddressSpaces] Handle the pair of `ptrtoint`/`inttoptr`.

Matt Arsenault via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 24 07:31:45 PDT 2020


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





More information about the cfe-commits mailing list