[llvm] r220277 - Teach the load analysis to allow finding available values which require
Hans Wennborg
hans at chromium.org
Tue Oct 21 17:00:51 PDT 2014
Revert is in r220349.
On Tue, Oct 21, 2014 at 4:53 PM, Reid Kleckner <rnk at google.com> wrote:
> It also caused http://llvm.org/pr21330, so I suspect Hans will it revert
> soon.
>
> On Tue, Oct 21, 2014 at 4:43 PM, Adam Nemet <anemet at apple.com> wrote:
>>
>> 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
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> 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