[InferAddressSpaces] Check if AS are the same in isNoopPtrIntCastPair

[wenju]

Summary

isNoopAddrSpaceCast is expecting SrcAS is different from DestAS.
If the two AS are the same, consider ptrtoint/inttoptr as noop cast.

[arsenm]

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

148–151

If the only point of this flag is this one test, it's probably not worth it.

808–809

This override won't override the target callback

llvm/test/Transforms/InferAddressSpaces/OpenCL/noop-ptrint-pair.ll

2 ↗

(On Diff #422132)

"OpenCL" test subdirectory doesn't make sense. The subdirectories are for setting the target build requirements for the tests

[wenju]

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

148–151

the issue is still reproducible using -assume-default-is-flat-addrspace flag, so I'll update

808–809

I see, thanks

llvm/test/Transforms/InferAddressSpaces/OpenCL/noop-ptrint-pair.ll

2 ↗

(On Diff #422132)

shall I put it in parent folder or X86 subfolder?

[wenju]

remove OverrideFlatAS, move test into X86 folder

[wenju]

@arsenm could you please submit for me? I don't have the permission. Thank you.

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

 static cl::opt<bool> AssumeDefaultIsFlatAddressSpace(
     "assume-default-is-flat-addrspace", cl::init(false), cl::ReallyHidden,
     cl::desc("The default address space is assumed as the flat address space. "
              "This is mainly for test purpose."));
 
 static const unsigned UninitializedAddressSpace =
     std::numeric_limits<unsigned>::max();
 
 namespace {
 
 using ValueToAddrSpaceMapTy = DenseMap<const Value *, unsigned>;
 // Different from ValueToAddrSpaceMapTy, where a new addrspace is inferred on
 // the *def* of a value, PredicatedAddrSpaceMapTy is map where a new
 // addrspace is inferred on the *use* of a pointer. This map is introduced to
 // infer addrspace from the addrspace predicate assumption built from assume
 // intrinsic. In that scenario, only specific uses (under valid assumption
 // context) could be inferred with a new addrspace.
 using PredicatedAddrSpaceMapTy =
     DenseMap<std::pair<const Value *, const Value *>, unsigned>;
 using PostorderStackTy = llvm::SmallVector<PointerIntPair<Value *, 1, bool>, 4>;
static bool isNoopPtrIntCastPair(const Operator *I2P, const DataLayout &DL,
   // The current IR spec doesn't have clear rules on address space casts,
   // especially a clear definition for pointer bits in non-default address
   // spaces. It would be undefined if that pointer is dereferenced after an
   // invalid reinterpret cast. Also, due to the unclearness for the meaning of
   // bits in non-default address spaces in the current spec, the pointer
   // arithmetic may also be undefined after invalid pointer reinterpret cast.
   // However, as we confirm through the target hooks that it's a no-op
   // addrspacecast, it doesn't matter since the bits should be the same.
+  unsigned P2IOp0AS = P2I->getOperand(0)->getType()->getPointerAddressSpace();
+  unsigned I2PAS = I2P->getType()->getPointerAddressSpace();
   return CastInst::isNoopCast(Instruction::CastOps(I2P->getOpcode()),
                               I2P->getOperand(0)->getType(), I2P->getType(),
                               DL) &&
          CastInst::isNoopCast(Instruction::CastOps(P2I->getOpcode()),
                               P2I->getOperand(0)->getType(), P2I->getType(),
                               DL) &&
-         TTI->isNoopAddrSpaceCast(
-             P2I->getOperand(0)->getType()->getPointerAddressSpace(),
-             I2P->getType()->getPointerAddressSpace());
+         (P2IOp0AS == I2PAS || TTI->isNoopAddrSpaceCast(P2IOp0AS, I2PAS));
 }
 
 // Returns true if V is an address expression.
 // TODO: Currently, we consider only phi, bitcast, addrspacecast, and
 // getelementptr operators.
 static bool isAddressExpression(const Value &V, const DataLayout &DL,
                                 const TargetTransformInfo *TTI) {
   const Operator *Op = dyn_cast<Operator>(&V);

 }
 
 bool InferAddressSpacesImpl::run(Function &F) {
   DL = &F.getParent()->getDataLayout();
 
   if (AssumeDefaultIsFlatAddressSpace)
     FlatAddrSpace = 0;
 
   if (FlatAddrSpace == UninitializedAddressSpace) {
     FlatAddrSpace = TTI->getFlatAddressSpace();
     if (FlatAddrSpace == UninitializedAddressSpace)
       return false;
   }
 
   // Collects all flat address expressions in postorder.
   std::vector<WeakTrackingVH> Postorder = collectFlatAddressExpressions(F);
 
   // Runs a data-flow analysis to refine the address spaces of every expression

llvm/test/Transforms/InferAddressSpaces/X86/noop-ptrint-pair.ll

+; RUN: opt -mtriple=x86_64-unknown-unknown -S -o - -infer-address-spaces -assume-default-is-flat-addrspace %s | FileCheck %s
+
+; Check that assert in X86TargetMachine::isNoopAddrSpaceCast is not triggered.
+
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-ni:7"
+
+; CHECK-LABEL: @noop_ptrint_pair(
+; CHECK: addrspacecast i32 addrspace(1)* %x to i32 addrspace(4)*
+; CHECK-NEXT: ptrtoint i32 addrspace(4)* %{{.*}} to i64
+; CHECK-NEXT: inttoptr i64 %{{.*}} to i32 addrspace(4)*
+define void @noop_ptrint_pair(i32 addrspace(1)* %x) {
+  %1 = addrspacecast i32 addrspace(1)* %x to i32 addrspace(4)*
+  %2 = ptrtoint i32 addrspace(4)* %1 to i64
+  %3 = inttoptr i64 %2 to i32 addrspace(4)*
+  ret void
+}