Index: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp =================================================================== --- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp +++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp @@ -1305,12 +1305,17 @@ // Expand the core addrec. If we need post-loop scaling, force it to // expand to an integer type to avoid the need for additional casting. Type *ExpandTy = PostLoopScale ? IntTy : STy; + // We can't use a pointer type for the addrec if the pointer type is + // non-integral. + Type *AddRecPHIExpandTy = + DL.isNonIntegralPointerType(STy) ? Normalized->getType() : ExpandTy; + // In some cases, we decide to reuse an existing phi node but need to truncate // it and/or invert the step. Type *TruncTy = nullptr; bool InvertStep = false; - PHINode *PN = getAddRecExprPHILiterally(Normalized, L, ExpandTy, IntTy, - TruncTy, InvertStep); + PHINode *PN = getAddRecExprPHILiterally(Normalized, L, AddRecPHIExpandTy, + IntTy, TruncTy, InvertStep); // Accommodate post-inc mode, if necessary. Value *Result; @@ -1383,8 +1388,15 @@ // Re-apply any non-loop-dominating offset. if (PostLoopOffset) { if (PointerType *PTy = dyn_cast(ExpandTy)) { - const SCEV *const OffsetArray[1] = { PostLoopOffset }; - Result = expandAddToGEP(OffsetArray, OffsetArray+1, PTy, IntTy, Result); + if (Result->getType()->isIntegerTy()) { + Value *Base = expandCodeFor(PostLoopOffset, ExpandTy); + const SCEV *const OffsetArray[1] = {SE.getUnknown(Result)}; + Result = expandAddToGEP(OffsetArray, OffsetArray + 1, PTy, IntTy, Base); + } else { + const SCEV *const OffsetArray[1] = {PostLoopOffset}; + Result = + expandAddToGEP(OffsetArray, OffsetArray + 1, PTy, IntTy, Result); + } } else { Result = InsertNoopCastOfTo(Result, IntTy); Result = Builder.CreateAdd(Result, Index: llvm/trunk/test/Transforms/LoopStrengthReduce/nonintegral.ll =================================================================== --- llvm/trunk/test/Transforms/LoopStrengthReduce/nonintegral.ll +++ llvm/trunk/test/Transforms/LoopStrengthReduce/nonintegral.ll @@ -0,0 +1,45 @@ +; RUN: opt -S -loop-reduce < %s | FileCheck %s + +; Address Space 10 is non-integral. The optimizer is not allowed to use +; ptrtoint/inttoptr instructions. Make sure that this doesn't happen +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:10:11:12" +target triple = "x86_64-unknown-linux-gnu" + +define void @japi1__unsafe_getindex_65028(i64 addrspace(10)* %arg) { +; CHECK-NOT: inttoptr +; CHECK-NOT: ptrtoint +; How exactly SCEV chooses to materialize isn't all that important, as +; long as it doesn't try to round-trip through integers. As of this writing, +; it emits a byte-wise gep, which is fine. +; CHECK: getelementptr i64, i64 addrspace(10)* {{.*}}, i64 {{.*}} +top: + br label %L86 + +L86: ; preds = %L86, %top + %i.0 = phi i64 [ 0, %top ], [ %tmp, %L86 ] + %tmp = add i64 %i.0, 1 + br i1 undef, label %L86, label %if29 + +if29: ; preds = %L86 + %tmp1 = shl i64 %tmp, 1 + %tmp2 = add i64 %tmp1, -2 + br label %if31 + +if31: ; preds = %if38, %if29 + %"#temp#1.sroa.0.022" = phi i64 [ 0, %if29 ], [ %tmp3, %if38 ] + br label %L119 + +L119: ; preds = %L119, %if31 + %i5.0 = phi i64 [ %"#temp#1.sroa.0.022", %if31 ], [ %tmp3, %L119 ] + %tmp3 = add i64 %i5.0, 1 + br i1 undef, label %L119, label %if38 + +if38: ; preds = %L119 + %tmp4 = add i64 %tmp2, %i5.0 + %tmp5 = getelementptr i64, i64 addrspace(10)* %arg, i64 %tmp4 + %tmp6 = load i64, i64 addrspace(10)* %tmp5 + br i1 undef, label %done, label %if31 + +done: ; preds = %if38 + ret void +} Index: llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp =================================================================== --- llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp +++ llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp @@ -7,21 +7,22 @@ // //===----------------------------------------------------------------------===// -#include "llvm/Analysis/ScalarEvolutionExpander.h" -#include "llvm/Analysis/ScalarEvolutionExpressions.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/AssumptionCache.h" #include "llvm/Analysis/LoopInfo.h" -#include "llvm/Analysis/TargetLibraryInfo.h" -#include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/LoopInfo.h" +#include "llvm/Analysis/ScalarEvolutionExpander.h" +#include "llvm/Analysis/ScalarEvolutionExpressions.h" +#include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/AsmParser/Parser.h" #include "llvm/IR/Constants.h" #include "llvm/IR/Dominators.h" #include "llvm/IR/GlobalVariable.h" +#include "llvm/IR/IRBuilder.h" #include "llvm/IR/InstIterator.h" #include "llvm/IR/LLVMContext.h" -#include "llvm/IR/Module.h" #include "llvm/IR/LegacyPassManager.h" +#include "llvm/IR/Module.h" #include "llvm/IR/Verifier.h" #include "llvm/Support/SourceMgr.h" #include "gtest/gtest.h" @@ -853,5 +854,82 @@ Type *I128Ty = Type::getInt128Ty(Context); SE.getZeroExtendExpr(S, I128Ty); } + +// Make sure that SCEV doesn't introduce illegal ptrtoint/inttoptr instructions +TEST_F(ScalarEvolutionsTest, SCEVZeroExtendExprNonIntegral) { + /* + * Create the following code: + * func(i64 addrspace(10)* %arg) + * top: + * br label %L.ph + * L.ph: + * br label %L + * L: + * %phi = phi i64 [i64 0, %L.ph], [ %add, %L2 ] + * %add = add i64 %phi2, 1 + * br i1 undef, label %post, label %L2 + * post: + * %gepbase = getelementptr i64 addrspace(10)* %arg, i64 1 + * #= %gep = getelementptr i64 addrspace(10)* %gepbase, i64 %add =# + * ret void + * + * We will create the appropriate SCEV expression for %gep and expand it, + * then check that no inttoptr/ptrtoint instructions got inserted. + */ + + // Create a module with non-integral pointers in it's datalayout + Module NIM("nonintegral", Context); + std::string DataLayout = M.getDataLayoutStr(); + if (!DataLayout.empty()) + DataLayout += "-"; + DataLayout += "ni:10"; + NIM.setDataLayout(DataLayout); + + Type *T_int1 = Type::getInt1Ty(Context); + Type *T_int64 = Type::getInt64Ty(Context); + Type *T_pint64 = T_int64->getPointerTo(10); + + FunctionType *FTy = + FunctionType::get(Type::getVoidTy(Context), {T_pint64}, false); + Function *F = cast(NIM.getOrInsertFunction("foo", FTy)); + + Argument *Arg = &*F->arg_begin(); + + BasicBlock *Top = BasicBlock::Create(Context, "top", F); + BasicBlock *LPh = BasicBlock::Create(Context, "L.ph", F); + BasicBlock *L = BasicBlock::Create(Context, "L", F); + BasicBlock *Post = BasicBlock::Create(Context, "post", F); + + IRBuilder<> Builder(Top); + Builder.CreateBr(LPh); + + Builder.SetInsertPoint(LPh); + Builder.CreateBr(L); + + Builder.SetInsertPoint(L); + PHINode *Phi = Builder.CreatePHI(T_int64, 2); + Value *Add = Builder.CreateAdd(Phi, ConstantInt::get(T_int64, 1), "add"); + Builder.CreateCondBr(UndefValue::get(T_int1), L, Post); + Phi->addIncoming(ConstantInt::get(T_int64, 0), LPh); + Phi->addIncoming(Add, L); + + Builder.SetInsertPoint(Post); + Value *GepBase = Builder.CreateGEP(Arg, {ConstantInt::get(T_int64, 1)}); + Instruction *Ret = Builder.CreateRetVoid(); + + ScalarEvolution SE = buildSE(*F); + auto *AddRec = + SE.getAddRecExpr(SE.getUnknown(GepBase), SE.getConstant(T_int64, 1), + LI->getLoopFor(L), SCEV::FlagNUW); + + SCEVExpander Exp(SE, NIM.getDataLayout(), "expander"); + Exp.disableCanonicalMode(); + Exp.expandCodeFor(AddRec, T_pint64, Ret); + + // Make sure none of the instructions inserted were inttoptr/ptrtoint. + // The verifier will check this. + EXPECT_FALSE(verifyFunction(*F, &errs())); +} + } // end anonymous namespace } // end namespace llvm