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