[llvm] r220277 - Teach the load analysis to allow finding available values which require
Reid Kleckner
rnk at google.com
Tue Oct 21 16:53:44 PDT 2014
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141021/244311da/attachment.html>
More information about the llvm-commits
mailing list