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

[Matt Arsenault via Phabricator via cfe-commits]

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
----------------
hliao wrote:
> arsenm wrote:
> > Positive checks are preferable (here and through the rest of the file)
> here, we need to ensure, after optimization, these values are promoted from memory
Yes, you can explicitly check for the instructions you expect, rather than ones you do not


================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:237-239
+         TTI->isNoopAddrSpaceCast(
+             P2I->getOperand(0)->getType()->getPointerAddressSpace(),
+             I2P->getType()->getPointerAddressSpace());
----------------
hliao wrote:
> arsenm wrote:
> > 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
> As we may convert a pair of ptr/int casts into an `addrspacecast` if both of them are no-op casts, if that's not a no-op `addrspacecast` under a specific target, we may introduce an invalid `addrspacecast` based on the current IR spec.
> 
I mean there needs to be comment explaining all of this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81938