[InstCombine] eliminate a pointer cast around insertelement

[spatel]

Summary

I'm not sure if this solves PR46839 completely, but reducing the casting should help:
https://bugs.llvm.org/show_bug.cgi?id=46839

I have not dealt much with pointer casts, so if anyone sees any corner-cases to test/avoid, please comment. Alive2 does not appear to support these ops.

[nikic]

Looks good to me. You already test the zext/trunc cases. I think this fold is also not problematic wrt https://bugs.llvm.org/show_bug.cgi?id=34548.

[efriedma]

If we're eliminating an inttoptr/ptrtoint pair, do we need to check that the pointer type is large enough to hold the integer type? (I don't know how likely this is to come up given instcombine canonicalizes the casts, but better to be safe, I think.)

[aqjune]

I agree this transformation is safe modulo the pointer size check. It should be fine to fold p2i(i2p i) -> i when the size of pointer is large enough.

[spatel]

Patch updated:
Given the transform above this one and the type checks included there and here, I think we can assert that sizes match. Let me know if that handles the possible problem patterns that we must avoid.

[efriedma]

Oh, I see, the transform to canonicalize the integer width always runs before. LGTM

[lebedev.ri]

LGTM, thank you!

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

 
 Instruction *InstCombinerImpl::visitPtrToInt(PtrToIntInst &CI) {
   // If the destination integer type is not the intptr_t type for this target,
   // do a ptrtoint to intptr_t then do a trunc or zext.  This allows the cast
   // to be exposed to other transforms.
   Value *SrcOp = CI.getPointerOperand();
   Type *Ty = CI.getType();
   unsigned AS = CI.getPointerAddressSpace();
-  if (Ty->getScalarSizeInBits() != DL.getPointerSizeInBits(AS)) {
+  unsigned TySize = Ty->getScalarSizeInBits();
+  unsigned PtrSize = DL.getPointerSizeInBits(AS);
+  if (TySize != PtrSize) {
     Type *IntPtrTy = DL.getIntPtrType(CI.getContext(), AS);
     if (auto *VecTy = dyn_cast<VectorType>(Ty)) {
       // Handle vectors of pointers.
       // FIXME: what should happen for scalable vectors?
       IntPtrTy = FixedVectorType::get(IntPtrTy, VecTy->getNumElements());
     }
 
     Value *P = Builder.CreatePtrToInt(SrcOp, IntPtrTy);
     return CastInst::CreateIntegerCast(P, Ty, /*isSigned=*/false);
   }
 
+  Value *Vec, *Scalar, *Index;
+  if (match(SrcOp, m_OneUse(m_InsertElt(m_IntToPtr(m_Value(Vec)),
+                                        m_Value(Scalar), m_Value(Index)))) &&
+      Vec->getType() == Ty) {
+    assert(Vec->getType()->getScalarSizeInBits() == PtrSize && "Wrong type");
+    // Convert the scalar to int followed by insert to eliminate one cast:
+    // p2i (ins (i2p Vec), Scalar, Index --> ins Vec, (p2i Scalar), Index
+    Value *NewCast = Builder.CreatePtrToInt(Scalar, Ty->getScalarType());
+    return InsertElementInst::Create(Vec, NewCast, Index);
+  }
+
   return commonPointerCastTransforms(CI);
 }
 
 /// This input value (which is known to have vector type) is being zero extended
 /// or truncated to the specified vector type. Since the zext/trunc is done
 /// using an integer type, we have a (bitcast(cast(bitcast))) pattern,
 /// endianness will impact which end of the vector that is extended or
 /// truncated.

llvm/test/Transforms/InstCombine/cast_ptr.ll

 entry:
   %arrayidx223 = addrspacecast i8 addrspace(1)* %source to i8*
   %t4 = load i8, i8* %arrayidx223
   ret i8 %t4
 }
 
 define <2 x i32> @insertelt(<2 x i32> %x, i32* %p, i133 %index) {
 ; CHECK-LABEL: @insertelt(
-; CHECK-NEXT:    [[V:%.*]] = inttoptr <2 x i32> [[X:%.*]] to <2 x i32*>
-; CHECK-NEXT:    [[I:%.*]] = insertelement <2 x i32*> [[V]], i32* [[P:%.*]], i133 [[INDEX:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = ptrtoint <2 x i32*> [[I]] to <2 x i32>
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i32* [[P:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = insertelement <2 x i32> [[X:%.*]], i32 [[TMP1]], i133 [[INDEX:%.*]]
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %v = inttoptr <2 x i32> %x to <2 x i32*>
   %i = insertelement <2 x i32*> %v, i32* %p, i133 %index
   %r = ptrtoint <2 x i32*> %i to <2 x i32>
   ret <2 x i32> %r
 }
 
 define <2 x i32> @insertelt_intptr_trunc(<2 x i64> %x, i32* %p) {
 ; CHECK-LABEL: @insertelt_intptr_trunc(
 ; CHECK-NEXT:    [[TMP1:%.*]] = trunc <2 x i64> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[V:%.*]] = inttoptr <2 x i32> [[TMP1]] to <2 x i32*>
-; CHECK-NEXT:    [[I:%.*]] = insertelement <2 x i32*> [[V]], i32* [[P:%.*]], i32 0
-; CHECK-NEXT:    [[R:%.*]] = ptrtoint <2 x i32*> [[I]] to <2 x i32>
+; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint i32* [[P:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = insertelement <2 x i32> [[TMP1]], i32 [[TMP2]], i32 0
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %v = inttoptr <2 x i64> %x to <2 x i32*>
   %i = insertelement <2 x i32*> %v, i32* %p, i32 0
   %r = ptrtoint <2 x i32*> %i to <2 x i32>
   ret <2 x i32> %r
 }
 
 define <2 x i32> @insertelt_intptr_zext(<2 x i8> %x, i32* %p) {
 ; CHECK-LABEL: @insertelt_intptr_zext(
 ; CHECK-NEXT:    [[TMP1:%.*]] = zext <2 x i8> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[V:%.*]] = inttoptr <2 x i32> [[TMP1]] to <2 x i32*>
-; CHECK-NEXT:    [[I:%.*]] = insertelement <2 x i32*> [[V]], i32* [[P:%.*]], i32 1
-; CHECK-NEXT:    [[R:%.*]] = ptrtoint <2 x i32*> [[I]] to <2 x i32>
+; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint i32* [[P:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = insertelement <2 x i32> [[TMP1]], i32 [[TMP2]], i32 1
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %v = inttoptr <2 x i8> %x to <2 x i32*>
   %i = insertelement <2 x i32*> %v, i32* %p, i32 1
   %r = ptrtoint <2 x i32*> %i to <2 x i32>
   ret <2 x i32> %r
 }
 
 define <2 x i64> @insertelt_intptr_zext_zext(<2 x i8> %x, i32* %p) {
 ; CHECK-LABEL: @insertelt_intptr_zext_zext(
 ; CHECK-NEXT:    [[TMP1:%.*]] = zext <2 x i8> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[V:%.*]] = inttoptr <2 x i32> [[TMP1]] to <2 x i32*>
-; CHECK-NEXT:    [[I:%.*]] = insertelement <2 x i32*> [[V]], i32* [[P:%.*]], i32 0
-; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint <2 x i32*> [[I]] to <2 x i32>
-; CHECK-NEXT:    [[R:%.*]] = zext <2 x i32> [[TMP2]] to <2 x i64>
+; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint i32* [[P:%.*]] to i32
+; CHECK-NEXT:    [[TMP3:%.*]] = insertelement <2 x i32> [[TMP1]], i32 [[TMP2]], i32 0
+; CHECK-NEXT:    [[R:%.*]] = zext <2 x i32> [[TMP3]] to <2 x i64>
 ; CHECK-NEXT:    ret <2 x i64> [[R]]
 ;
   %v = inttoptr <2 x i8> %x to <2 x i32*>
   %i = insertelement <2 x i32*> %v, i32* %p, i32 0
   %r = ptrtoint <2 x i32*> %i to <2 x i64>
   ret <2 x i64> %r
 }
 
 declare void @use(<2 x i32*>)
 
 define <2 x i32> @insertelt_extra_use1(<2 x i32> %x, i32* %p) {
 ; CHECK-LABEL: @insertelt_extra_use1(
 ; CHECK-NEXT:    [[V:%.*]] = inttoptr <2 x i32> [[X:%.*]] to <2 x i32*>
 ; CHECK-NEXT:    call void @use(<2 x i32*> [[V]])
-; CHECK-NEXT:    [[I:%.*]] = insertelement <2 x i32*> [[V]], i32* [[P:%.*]], i32 0
-; CHECK-NEXT:    [[R:%.*]] = ptrtoint <2 x i32*> [[I]] to <2 x i32>
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i32* [[P:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = insertelement <2 x i32> [[X]], i32 [[TMP1]], i32 0
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %v = inttoptr <2 x i32> %x to <2 x i32*>
   call void @use(<2 x i32*> %v)
   %i = insertelement <2 x i32*> %v, i32* %p, i32 0
   %r = ptrtoint <2 x i32*> %i to <2 x i32>
   ret <2 x i32> %r
 }