[llvm] r220277 - Teach the load analysis to allow finding available values which require
Adam Nemet
anemet at apple.com
Tue Oct 21 16:43:42 PDT 2014
Hi Chandler,
This commit causes xalancbmk to be miscompiled on ARMv7. Do you have access to SPEC 2006 and ARM?
Thanks,
Adam
On Oct 21, 2014, at 2:00 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Tue Oct 21 04:00:40 2014
> New Revision: 220277
>
> URL: http://llvm.org/viewvc/llvm-project?rev=220277&view=rev
> Log:
> Teach the load analysis to allow finding available values which require
> inttoptr or ptrtoint cast provided there is datalayout available.
> Eventually, the datalayout can just be required but in practice it will
> always be there today.
>
> To go with the ability to expose available values requiring a ptrtoint
> or inttoptr cast, helpers are added to perform one of these three casts.
>
> These smarts are necessary to finish canonicalizing loads and stores to
> the operational type requirements without regressing fundamental
> combines.
>
> I've added some test cases. These should actually improve as the load
> combining and store combining improves, but they may fundamentally be
> highlighting some missing combines for select in addition to exercising
> the specific added logic to load analysis.
>
> Modified:
> llvm/trunk/include/llvm/IR/IRBuilder.h
> llvm/trunk/include/llvm/IR/InstrTypes.h
> llvm/trunk/lib/Analysis/Loads.cpp
> llvm/trunk/lib/IR/Instructions.cpp
> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> llvm/trunk/test/Transforms/InstCombine/select.ll
>
> Modified: llvm/trunk/include/llvm/IR/IRBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/IRBuilder.h (original)
> +++ llvm/trunk/include/llvm/IR/IRBuilder.h Tue Oct 21 04:00:40 2014
> @@ -1246,6 +1246,18 @@ public:
> return Insert(Folder.CreateIntCast(VC, DestTy, isSigned), Name);
> return Insert(CastInst::CreateIntegerCast(V, DestTy, isSigned), Name);
> }
> +
> + Value *CreateBitOrPointerCast(Value *V, Type *DestTy,
> + const Twine &Name = "") {
> + if (V->getType() == DestTy)
> + return V;
> + if (V->getType()->isPointerTy() && DestTy->isIntegerTy())
> + return CreatePtrToInt(V, DestTy, Name);
> + if (V->getType()->isIntegerTy() && DestTy->isPointerTy())
> + return CreateIntToPtr(V, DestTy, Name);
> +
> + return CreateBitCast(V, DestTy, Name);
> + }
> private:
> // \brief Provided to resolve 'CreateIntCast(Ptr, Ptr, "...")', giving a
> // compile time error, instead of converting the string to bool for the
>
> Modified: llvm/trunk/include/llvm/IR/InstrTypes.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/InstrTypes.h?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/InstrTypes.h (original)
> +++ llvm/trunk/include/llvm/IR/InstrTypes.h Tue Oct 21 04:00:40 2014
> @@ -490,6 +490,19 @@ public:
> Instruction *InsertBefore = 0 ///< Place to insert the instruction
> );
>
> + /// @brief Create a BitCast, a PtrToInt, or an IntToPTr cast instruction.
> + ///
> + /// If the value is a pointer type and the destination an integer type,
> + /// creates a PtrToInt cast. If the value is an integer type and the
> + /// destination a pointer type, creates an IntToPtr cast. Otherwise, creates
> + /// a bitcast.
> + static CastInst *CreateBitOrPointerCast(
> + Value *S, ///< The pointer value to be casted (operand 0)
> + Type *Ty, ///< The type to which cast should be made
> + const Twine &Name = "", ///< Name for the instruction
> + Instruction *InsertBefore = 0 ///< Place to insert the instruction
> + );
> +
> /// @brief Create a ZExt, BitCast, or Trunc for int -> int casts.
> static CastInst *CreateIntegerCast(
> Value *S, ///< The pointer value to be casted (operand 0)
> @@ -552,6 +565,17 @@ public:
> Type *DestTy ///< The Type to which the value should be cast.
> );
>
> + /// @brief Check whether a bitcast, inttoptr, or ptrtoint cast between these
> + /// types is valid and a no-op.
> + ///
> + /// This ensures that any pointer<->integer cast has enough bits in the
> + /// integer and any other cast is a bitcast.
> + static bool isBitOrNoopPointerCastable(
> + Type *SrcTy, ///< The Type from which the value should be cast.
> + Type *DestTy, ///< The Type to which the value should be cast.
> + const DataLayout *Layout = 0 ///< Optional DataLayout.
> + );
> +
> /// Returns the opcode necessary to cast Val into Ty using usual casting
> /// rules.
> /// @brief Infer the opcode for cast operand and type
>
> Modified: llvm/trunk/lib/Analysis/Loads.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Loads.cpp?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/Loads.cpp (original)
> +++ llvm/trunk/lib/Analysis/Loads.cpp Tue Oct 21 04:00:40 2014
> @@ -176,8 +176,13 @@ Value *llvm::FindAvailableLoadedValue(Va
>
> Type *AccessTy = cast<PointerType>(Ptr->getType())->getElementType();
>
> - // If we're using alias analysis to disambiguate get the size of *Ptr.
> - uint64_t AccessSize = AA ? AA->getTypeStoreSize(AccessTy) : 0;
> + // Try to get the DataLayout for this module. This may be null, in which case
> + // the optimizations will be limited.
> + const DataLayout *DL = ScanBB->getDataLayout();
> +
> + // Try to get the store size for the type.
> + uint64_t AccessSize = DL ? DL->getTypeStoreSize(AccessTy)
> + : AA ? AA->getTypeStoreSize(AccessTy) : 0;
>
> Value *StrippedPtr = Ptr->stripPointerCasts();
>
> @@ -202,7 +207,7 @@ Value *llvm::FindAvailableLoadedValue(Va
> if (LoadInst *LI = dyn_cast<LoadInst>(Inst))
> if (AreEquivalentAddressValues(
> LI->getPointerOperand()->stripPointerCasts(), StrippedPtr) &&
> - CastInst::isBitCastable(LI->getType(), AccessTy)) {
> + CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
> if (AATags)
> LI->getAAMetadata(*AATags);
> return LI;
> @@ -214,7 +219,8 @@ Value *llvm::FindAvailableLoadedValue(Va
> // (This is true even if the store is volatile or atomic, although
> // those cases are unlikely.)
> if (AreEquivalentAddressValues(StorePtr, StrippedPtr) &&
> - CastInst::isBitCastable(SI->getValueOperand()->getType(), AccessTy)) {
> + CastInst::isBitOrNoopPointerCastable(SI->getValueOperand()->getType(),
> + AccessTy, DL)) {
> if (AATags)
> SI->getAAMetadata(*AATags);
> return SI->getOperand(0);
>
> Modified: llvm/trunk/lib/IR/Instructions.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instructions.cpp?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Instructions.cpp (original)
> +++ llvm/trunk/lib/IR/Instructions.cpp Tue Oct 21 04:00:40 2014
> @@ -2559,6 +2559,17 @@ CastInst *CastInst::CreatePointerBitCast
> return Create(Instruction::BitCast, S, Ty, Name, InsertBefore);
> }
>
> +CastInst *CastInst::CreateBitOrPointerCast(Value *S, Type *Ty,
> + const Twine &Name,
> + Instruction *InsertBefore) {
> + if (S->getType()->isPointerTy() && Ty->isIntegerTy())
> + return Create(Instruction::PtrToInt, S, Ty, Name, InsertBefore);
> + if (S->getType()->isIntegerTy() && Ty->isPointerTy())
> + return Create(Instruction::IntToPtr, S, Ty, Name, InsertBefore);
> +
> + return Create(Instruction::BitCast, S, Ty, Name, InsertBefore);
> +}
> +
> CastInst *CastInst::CreateIntegerCast(Value *C, Type *Ty,
> bool isSigned, const Twine &Name,
> Instruction *InsertBefore) {
> @@ -2716,6 +2727,18 @@ bool CastInst::isBitCastable(Type *SrcTy
> return true;
> }
>
> +bool CastInst::isBitOrNoopPointerCastable(Type *SrcTy, Type *DestTy,
> + const DataLayout *DL) {
> + if (auto *PtrTy = dyn_cast<PointerType>(SrcTy))
> + if (auto *IntTy = dyn_cast<IntegerType>(DestTy))
> + return DL && IntTy->getBitWidth() >= DL->getPointerTypeSizeInBits(PtrTy);
> + if (auto *PtrTy = dyn_cast<PointerType>(DestTy))
> + if (auto *IntTy = dyn_cast<IntegerType>(SrcTy))
> + return DL && IntTy->getBitWidth() >= DL->getPointerTypeSizeInBits(PtrTy);
> +
> + return isBitCastable(SrcTy, DestTy);
> +}
> +
> // Provide a way to get a "cast" where the cast opcode is inferred from the
> // types and size of the operand. This, basically, is a parallel of the
> // logic in the castIsValid function below. This axiom should hold:
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Tue Oct 21 04:00:40 2014
> @@ -417,7 +417,8 @@ Instruction *InstCombiner::visitLoadInst
> BasicBlock::iterator BBI = &LI;
> if (Value *AvailableVal = FindAvailableLoadedValue(Op, LI.getParent(), BBI,6))
> return ReplaceInstUsesWith(
> - LI, Builder->CreateBitCast(AvailableVal, LI.getType()));
> + LI, Builder->CreateBitOrPointerCast(AvailableVal, LI.getType(),
> + LI.getName() + ".cast"));
>
> // load(gep null, ...) -> unreachable
> if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
>
> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue Oct 21 04:00:40 2014
> @@ -902,8 +902,8 @@ bool JumpThreading::SimplifyPartiallyRed
> // only happen in dead loops.
> if (AvailableVal == LI) AvailableVal = UndefValue::get(LI->getType());
> if (AvailableVal->getType() != LI->getType())
> - AvailableVal = CastInst::Create(CastInst::BitCast, AvailableVal,
> - LI->getType(), "", LI);
> + AvailableVal =
> + CastInst::CreateBitOrPointerCast(AvailableVal, LI->getType(), "", LI);
> LI->replaceAllUsesWith(AvailableVal);
> LI->eraseFromParent();
> return true;
> @@ -1040,8 +1040,8 @@ bool JumpThreading::SimplifyPartiallyRed
> // predecessor use the same bitcast.
> Value *&PredV = I->second;
> if (PredV->getType() != LI->getType())
> - PredV = CastInst::Create(CastInst::BitCast, PredV, LI->getType(), "",
> - P->getTerminator());
> + PredV = CastInst::CreateBitOrPointerCast(PredV, LI->getType(), "",
> + P->getTerminator());
>
> PN->addIncoming(PredV, I->first);
> }
>
> Modified: llvm/trunk/test/Transforms/InstCombine/select.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select.ll?rev=220277&r1=220276&r2=220277&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/select.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/select.ll Tue Oct 21 04:00:40 2014
> @@ -1256,7 +1256,7 @@ define i32 @test76(i1 %flag, i32* %x) {
> ret i32 %v
> }
>
> -declare void @scribble_on_memory(i32*)
> +declare void @scribble_on_i32(i32*)
>
> define i32 @test77(i1 %flag, i32* %x) {
> ; The load here must not be speculated around the select. One side of the
> @@ -1264,13 +1264,13 @@ define i32 @test77(i1 %flag, i32* %x) {
> ; load does.
> ; CHECK-LABEL: @test77(
> ; CHECK: %[[A:.*]] = alloca i32, align 1
> -; CHECK: call void @scribble_on_memory(i32* %[[A]])
> +; CHECK: call void @scribble_on_i32(i32* %[[A]])
> ; CHECK: store i32 0, i32* %x
> ; CHECK: %[[P:.*]] = select i1 %flag, i32* %[[A]], i32* %x
> ; CHECK: load i32* %[[P]]
>
> %under_aligned = alloca i32, align 1
> - call void @scribble_on_memory(i32* %under_aligned)
> + call void @scribble_on_i32(i32* %under_aligned)
> store i32 0, i32* %x
> %p = select i1 %flag, i32* %under_aligned, i32* %x
> %v = load i32* %p
> @@ -1327,8 +1327,8 @@ define i32 @test80(i1 %flag) {
> entry:
> %x = alloca i32
> %y = alloca i32
> - call void @scribble_on_memory(i32* %x)
> - call void @scribble_on_memory(i32* %y)
> + call void @scribble_on_i32(i32* %x)
> + call void @scribble_on_i32(i32* %y)
> %tmp = load i32* %x
> store i32 %tmp, i32* %y
> %p = select i1 %flag, i32* %x, i32* %y
> @@ -1351,8 +1351,8 @@ entry:
> %y = alloca i32
> %x1 = bitcast float* %x to i32*
> %y1 = bitcast i32* %y to float*
> - call void @scribble_on_memory(i32* %x1)
> - call void @scribble_on_memory(i32* %y)
> + call void @scribble_on_i32(i32* %x1)
> + call void @scribble_on_i32(i32* %y)
> %tmp = load i32* %x1
> store i32 %tmp, i32* %y
> %p = select i1 %flag, float* %x, float* %y1
> @@ -1377,11 +1377,63 @@ entry:
> %y = alloca i32
> %x1 = bitcast float* %x to i32*
> %y1 = bitcast i32* %y to float*
> - call void @scribble_on_memory(i32* %x1)
> - call void @scribble_on_memory(i32* %y)
> + call void @scribble_on_i32(i32* %x1)
> + call void @scribble_on_i32(i32* %y)
> %tmp = load float* %x
> store float %tmp, float* %y1
> %p = select i1 %flag, i32* %x1, i32* %y
> %v = load i32* %p
> ret i32 %v
> }
> +
> +declare void @scribble_on_i64(i64*)
> +
> +define i8* @test83(i1 %flag) {
> +; Test that we can speculate the load around the select even though they use
> +; differently typed pointers and requires inttoptr casts.
> +; CHECK-LABEL: @test83(
> +; CHECK: %[[X:.*]] = alloca i8*
> +; CHECK-NEXT: %[[Y:.*]] = alloca i8*
> +; CHECK: %[[V:.*]] = load i64* %[[X]]
> +; CHECK-NEXT: %[[C1:.*]] = inttoptr i64 %[[V]] to i8*
> +; CHECK-NEXT: store i8* %[[C1]], i8** %[[Y]]
> +; CHECK-NEXT: %[[C2:.*]] = inttoptr i64 %[[V]] to i8*
> +; CHECK-NEXT: %[[S:.*]] = select i1 %flag, i8* %[[C2]], i8* %[[C1]]
> +; CHECK-NEXT: ret i8* %[[S]]
> +entry:
> + %x = alloca i8*
> + %y = alloca i64
> + %x1 = bitcast i8** %x to i64*
> + %y1 = bitcast i64* %y to i8**
> + call void @scribble_on_i64(i64* %x1)
> + call void @scribble_on_i64(i64* %y)
> + %tmp = load i64* %x1
> + store i64 %tmp, i64* %y
> + %p = select i1 %flag, i8** %x, i8** %y1
> + %v = load i8** %p
> + ret i8* %v
> +}
> +
> +define i64 @test84(i1 %flag) {
> +; Test that we can speculate the load around the select even though they use
> +; differently typed pointers and requires a ptrtoint cast.
> +; CHECK-LABEL: @test84(
> +; CHECK: %[[X:.*]] = alloca i8*
> +; CHECK-NEXT: %[[Y:.*]] = alloca i8*
> +; CHECK: %[[V:.*]] = load i8** %[[X]]
> +; CHECK-NEXT: store i8* %[[V]], i8** %[[Y]]
> +; CHECK-NEXT: %[[C:.*]] = ptrtoint i8* %[[V]] to i64
> +; CHECK-NEXT: ret i64 %[[C]]
> +entry:
> + %x = alloca i8*
> + %y = alloca i64
> + %x1 = bitcast i8** %x to i64*
> + %y1 = bitcast i64* %y to i8**
> + call void @scribble_on_i64(i64* %x1)
> + call void @scribble_on_i64(i64* %y)
> + %tmp = load i8** %x
> + store i8* %tmp, i8** %y1
> + %p = select i1 %flag, i64* %x1, i64* %y
> + %v = load i64* %p
> + ret i64 %v
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list