[PATCH] Don't constant fold bitcast/ptrtoint/inttoptr combinations without datalayout
Eli Friedman
eli.friedman at gmail.com
Tue Jul 16 16:41:25 PDT 2013
On Tue, Jul 16, 2013 at 11:57 AM, Matt Arsenault
<Matthew.Arsenault at amd.com> wrote:
> Hi eli.friedman,
>
> This is necessary to check for constantexpr inttoptr -> illegal bitcasts required for http://llvm-reviews.chandlerc.com/D1083
>
> For some reason, constant folding happens before the verifier is reached, so my tests for rejecting those illegal bitcasts in this case can't work.
>
> This change also causes a test to hit the bug that is fixed by http://llvm-reviews.chandlerc.com/D1114
>
> As it is right now, this introduces one inline cost test failure, which I believe will be fixed by another larger patch I have that I was going to submit later that adds the address space support there.
Which test is this? It's likely to be a bug in the test if it's
depending on our behavior here.
Please check that FoldBitCast in lib/Analysis/ConstantFolding.cpp does
the right thing.
Patch looks fine.
-Eli
> http://llvm-reviews.chandlerc.com/D1157
>
> Files:
> lib/IR/ConstantFold.cpp
> lib/IR/Instructions.cpp
> unittests/IR/InstructionsTest.cpp
>
> Index: lib/IR/ConstantFold.cpp
> ===================================================================
> --- lib/IR/ConstantFold.cpp
> +++ lib/IR/ConstantFold.cpp
> @@ -87,13 +87,9 @@
> Instruction::CastOps firstOp = Instruction::CastOps(Op->getOpcode());
> Instruction::CastOps secondOp = Instruction::CastOps(opc);
>
> - // Assume that pointers are never more than 64 bits wide.
> - IntegerType *FakeIntPtrTy = Type::getInt64Ty(DstTy->getContext());
> -
> // Let CastInst::isEliminableCastPair do the heavy lifting.
> return CastInst::isEliminableCastPair(firstOp, secondOp, SrcTy, MidTy, DstTy,
> - FakeIntPtrTy, FakeIntPtrTy,
> - FakeIntPtrTy);
> + 0, 0, 0);
> }
>
> static Constant *FoldBitCast(Constant *V, Type *DestTy) {
> Index: lib/IR/Instructions.cpp
> ===================================================================
> --- lib/IR/Instructions.cpp
> +++ lib/IR/Instructions.cpp
> @@ -2224,12 +2224,20 @@
> if (SrcTy->isFloatingPointTy())
> return secondOp;
> return 0;
> - case 7: {
> - // ptrtoint, inttoptr -> bitcast (ptr -> ptr) if int size is >= ptr size
> + case 7: {
> + unsigned MidSize = MidTy->getScalarSizeInBits();
> + // Check the address spaces first. If we know they are in the same address
> + // space, the pointer sizes must be the same so we can still fold this
> + // without knowing the actual sizes as long we know that the intermediate
> + // pointer is the largest possible pointer size.
> + if (MidSize == 64 &&
> + SrcTy->getPointerAddressSpace() == DstTy->getPointerAddressSpace())
> + return Instruction::BitCast;
> +
> + // ptrtoint, inttoptr -> bitcast (ptr -> ptr) if int size is >= ptr size.
> if (!SrcIntPtrTy || DstIntPtrTy != SrcIntPtrTy)
> return 0;
> unsigned PtrSize = SrcIntPtrTy->getScalarSizeInBits();
> - unsigned MidSize = MidTy->getScalarSizeInBits();
> if (MidSize >= PtrSize)
> return Instruction::BitCast;
> return 0;
> @@ -2257,12 +2265,23 @@
> case 11:
> // bitcast followed by ptrtoint is allowed as long as the bitcast
> // is a pointer to pointer cast.
> - if (SrcTy->isPointerTy() && MidTy->isPointerTy())
> + if (SrcTy->isPointerTy() &&
> + MidTy->isPointerTy() &&
> + SrcIntPtrTy &&
> + DstIntPtrTy &&
> + SrcIntPtrTy->getScalarSizeInBits() ==
> + DstIntPtrTy->getScalarSizeInBits())
> return secondOp;
> return 0;
> case 12:
> // inttoptr, bitcast -> intptr if bitcast is a ptr to ptr cast
> - if (MidTy->isPointerTy() && DstTy->isPointerTy())
> + // and the ptrs are to address spaces of the same size
> + if (MidTy->isPointerTy() &&
> + DstTy->isPointerTy() &&
> + MidIntPtrTy &&
> + DstIntPtrTy &&
> + MidIntPtrTy->getScalarSizeInBits() ==
> + DstIntPtrTy->getScalarSizeInBits())
> return firstOp;
> return 0;
> case 13: {
> Index: unittests/IR/InstructionsTest.cpp
> ===================================================================
> --- unittests/IR/InstructionsTest.cpp
> +++ unittests/IR/InstructionsTest.cpp
> @@ -255,6 +255,7 @@
> TEST(InstructionsTest, isEliminableCastPair) {
> LLVMContext &C(getGlobalContext());
>
> + Type* Int16Ty = Type::getInt16Ty(C);
> Type* Int32Ty = Type::getInt32Ty(C);
> Type* Int64Ty = Type::getInt64Ty(C);
> Type* Int64PtrTy = Type::getInt64PtrTy(C);
> @@ -266,11 +267,20 @@
> Int32Ty, 0, Int32Ty),
> CastInst::BitCast);
>
> - // Source and destination pointers have different sizes -> fail.
> + // Source and destination have unknown sizes, but the same address space and
> + // the intermediate int is the maximum pointer size -> bitcast
> EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::PtrToInt,
> CastInst::IntToPtr,
> Int64PtrTy, Int64Ty, Int64PtrTy,
> - Int32Ty, 0, Int64Ty),
> + 0, 0, 0),
> + CastInst::BitCast);
> +
> + // Source and destination have unknown sizes, but the same address space and
> + // the intermediate int is not the maximum pointer size -> nothing
> + EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::PtrToInt,
> + CastInst::IntToPtr,
> + Int64PtrTy, Int32Ty, Int64PtrTy,
> + 0, 0, 0),
> 0U);
>
> // Middle pointer big enough -> bitcast.
> @@ -286,7 +296,61 @@
> Int64Ty, Int64PtrTy, Int64Ty,
> 0, Int32Ty, 0),
> 0U);
> +
> +
> + // Test that we don't eliminate bitcasts between different address spaces,
> + // or if we don't have available pointer size information.
> + DataLayout DL("e-p:32:32:32-p1:16:16:16-p2:64:64:64-i1:8:8-i8:8:8-i16:16:16"
> + "-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64"
> + "-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128");
> +
> + Type* Int64PtrTyAS1 = Type::getInt64PtrTy(C, 1);
> + Type* Int64PtrTyAS2 = Type::getInt64PtrTy(C, 2);
> +
> + IntegerType *Int16SizePtr = DL.getIntPtrType(C, 1);
> + IntegerType *Int64SizePtr = DL.getIntPtrType(C, 2);
> +
> + // Fail since the ptr int types are not provided
> + EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::IntToPtr,
> + CastInst::BitCast,
> + Int16Ty, Int64PtrTyAS1, Int64PtrTyAS2,
> + 0, 0, 0),
> + 0U);
> +
> + // Fail since the the bitcast is between different sized address spaces
> + EXPECT_EQ(CastInst::isEliminableCastPair(
> + CastInst::IntToPtr,
> + CastInst::BitCast,
> + Int16Ty, Int64PtrTyAS1, Int64PtrTyAS2,
> + 0, Int16SizePtr, Int64SizePtr),
> + 0U);
> +
> +
> + // Fail since the the bitcast is between different sized address spaces
> + EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::IntToPtr,
> + CastInst::BitCast,
> + Int16Ty, Int64PtrTyAS1, Int64PtrTyAS2,
> + 0, Int16SizePtr, Int64SizePtr),
> + 0U);
> +
> +
> +
> + // Fail without known pointer sizes
> + EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::BitCast,
> + CastInst::PtrToInt,
> + Int16Ty, Int64PtrTyAS1, Int64PtrTyAS2,
> + 0, 0, 0),
> + 0U);
> +
> + // Fail since the bitcast is the wrong size
> + EXPECT_EQ(CastInst::isEliminableCastPair(CastInst::BitCast,
> + CastInst::PtrToInt,
> + Int64PtrTyAS1, Int64PtrTyAS2, Int64Ty,
> + Int16SizePtr, Int64SizePtr, 0),
> + 0U);
> }
>
> } // end anonymous namespace
> } // end namespace llvm
> +
> +
More information about the llvm-commits
mailing list