[llvm] r220277 - Teach the load analysis to allow finding available values which require
Chandler Carruth
chandlerc at google.com
Tue Oct 21 17:12:43 PDT 2014
Thanks folks and sorry. Not at a computer.
Also frustrating considering I actually ran LNT over it without issue.
On Oct 21, 2014 5:04 PM, "Hans Wennborg" <hans at chromium.org> wrote:
> 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
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141021/0eb40705/attachment.html>
More information about the llvm-commits
mailing list