Handle ptrtoint in InferAddressSpace

[gangche1]

Summary

We see in our IR that has addrspacecast before ptrtoint, which requires this optimization. Therefore suggest to add the handling of ptrtoint.

[gangche1]

We see in our IR that has addrspacecast before ptrtoint, which requires this optimization. Therefore suggest to add the handling of ptrtoint.

[arsenm]

This is incorrect and assumes the address spaces have equivalent bit representations

[hliao]

inttoptr and ptrtoint should be treated as opaque ones. If needed, they should be handled with target-specific. This pass is definitely not the place to handle them.

In D72941#1827075, @gangche1 wrote:

We see in our IR that has addrspacecast before ptrtoint, which requires this optimization. Therefore suggest to add the handling of ptrtoint.

[arsenm]

In D72941#1828007, @hliao wrote:

inttoptr and ptrtoint should be treated as opaque ones. If needed, they should be handled with target-specific. This pass is definitely not the place to handle them.

In D72941#1827075, @gangche1 wrote:

We see in our IR that has addrspacecast before ptrtoint, which requires this optimization. Therefore suggest to add the handling of ptrtoint.

This is backwards, inttoptr/ptrtoint are "copies" that may truncate or trivially extend. addrspacecast is the opaque part

[hliao]

In D72941#1828060, @arsenm wrote:

In D72941#1828007, @hliao wrote:

inttoptr and ptrtoint should be treated as opaque ones. If needed, they should be handled with target-specific. This pass is definitely not the place to handle them.

In D72941#1827075, @gangche1 wrote:

We see in our IR that has addrspacecast before ptrtoint, which requires this optimization. Therefore suggest to add the handling of ptrtoint.

This is backwards, inttoptr/ptrtoint are "copies" that may truncate or trivially extend. addrspacecast is the opaque part

Thanks for correcting me. What I really mean is that that "copies" are opaque and you cannot interpret them the same as addrspacecast. They are different in semantics.

[gangche1]

What if I add another query in TTI, check if an address space has the same bit-pattern as the flat-address, then handle the ptrtoint with this extra condition?

[arsenm]

In D72941#1832580, @gangche1 wrote:

What if I add another query in TTI, check if an address space has the same bit-pattern as the flat-address, then handle the ptrtoint with this extra condition?

What is the problem you are really trying to solve? There's already a isNoopAddrspaceCast hook

[gangche1]

isNoopAddrspaceCast is what I am looking for. Could I use that in this InferAddressSpace pass to get rid of a NoopAddrSpaceCast that is used by ptrtoint?

[arsenm]

In D72941#1832619, @gangche1 wrote:

isNoopAddrspaceCast is what I am looking for. Could I use that in this InferAddressSpace pass to get rid of a NoopAddrSpaceCast that is used by ptrtoint?

Maybe, but I'm wondering why you have noop addrspacecasts in the first place. This seems like a problem you shouldn't have to solve

[gangche1]

how do I withdraw this review?

[arsenm]

In D72941#1834753, @gangche1 wrote:

how do I withdraw this review?

There's an abandon revision. We also now do recognize certain ptrtoint/inttoptr pairs, although this is somewhat questionable

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

       PushPtrOperand(LI->getPointerOperand());
     else if (auto *SI = dyn_cast<StoreInst>(&I))
       PushPtrOperand(SI->getPointerOperand());
+    else if (auto *P2I = dyn_cast<PtrToIntInst>(&I))
+      PushPtrOperand(P2I->getPointerOperand());
     else if (auto *RMW = dyn_cast<AtomicRMWInst>(&I))
       PushPtrOperand(RMW->getPointerOperand());
     else if (auto *CmpX = dyn_cast<AtomicCmpXchgInst>(&I))

     return OpNo == StoreInst::getPointerOperandIndex() &&
            (VolatileIsAllowed || !SI->isVolatile());
 
+  if (auto *P2I = dyn_cast<PtrToIntInst>(Inst))
+    return OpNo == PtrToIntInst::getPointerOperandIndex();
+
   if (auto *RMW = dyn_cast<AtomicRMWInst>(Inst))
     return OpNo == AtomicRMWInst::getPointerOperandIndex() &&
            (VolatileIsAllowed || !RMW->isVolatile());

llvm/test/Transforms/InferAddressSpaces/AMDGPU/ptrtoint.ll

+; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -infer-address-spaces %s | FileCheck %s
+
+; Optimized to private ptrtoint.
+; CHECK-LABEL: @ptrtoint_private(
+; CHECK-NEXT: %val = ptrtoint i32 addrspace(5)* %input to i32
+define i32 @ptrtoint_private(i32 addrspace(5)* nocapture %input) #0 {
+  %tmp0 = addrspacecast i32 addrspace(5)* %input to i32*
+  %val =  ptrtoint i32* %tmp0 to i32
+  ret i32 %val
+}
+
+attributes #0 = { nounwind }