forked from OSchip/llvm-project
Revert "[SCEVExpand] do not hoist divisions by zero (PR30935)"
Reverts r289412. It caused an OOB PHI operand access in instcombine when ASan is enabled. Reduction in progress. Also reverts "[SCEVExpander] Add a test case related to r289412" llvm-svn: 289453
This commit is contained in:
Reid Kleckner
parent
4881bdf141
commit
30422eea0f
|
|
@ -2130,7 +2130,7 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
|
|||
}
|
||||
|
||||
// Okay, check to see if the same value occurs in the operand list more than
|
||||
// once. If so, merge them together into a multiply expression. Since we
|
||||
// once. If so, merge them together into an multiply expression. Since we
|
||||
// sorted the list, these values are required to be adjacent.
|
||||
Type *Ty = Ops[0]->getType();
|
||||
bool FoundMatch = false;
|
||||
|
|
|
|||
|
|
@ -166,16 +166,6 @@ Value *SCEVExpander::InsertNoopCastOfTo(Value *V, Type *Ty) {
|
|||
return ReuseOrCreateCast(I, Ty, Op, IP);
|
||||
}
|
||||
|
||||
// Return true when S may contain the value zero.
|
||||
static bool mayBeValueZero(Value *V) {
|
||||
if (ConstantInt *C = dyn_cast<ConstantInt>(V))
|
||||
if (!C->isZero())
|
||||
return false;
|
||||
|
||||
// All other expressions may have a zero value.
|
||||
return true;
|
||||
}
|
||||
|
||||
/// InsertBinop - Insert the specified binary operator, doing a small amount
|
||||
/// of work to avoid inserting an obviously redundant operation.
|
||||
Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode,
|
||||
|
|
@ -208,17 +198,14 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode,
|
|||
DebugLoc Loc = Builder.GetInsertPoint()->getDebugLoc();
|
||||
SCEVInsertPointGuard Guard(Builder, this);
|
||||
|
||||
// Only move the insertion point up when it is not a division by zero.
|
||||
if (Opcode != Instruction::UDiv || !mayBeValueZero(RHS)) {
|
||||
// Move the insertion point out of as many loops as we can.
|
||||
while (const Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock())) {
|
||||
if (!L->isLoopInvariant(LHS) || !L->isLoopInvariant(RHS)) break;
|
||||
BasicBlock *Preheader = L->getLoopPreheader();
|
||||
if (!Preheader) break;
|
||||
// Move the insertion point out of as many loops as we can.
|
||||
while (const Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock())) {
|
||||
if (!L->isLoopInvariant(LHS) || !L->isLoopInvariant(RHS)) break;
|
||||
BasicBlock *Preheader = L->getLoopPreheader();
|
||||
if (!Preheader) break;
|
||||
|
||||
// Ok, move up a level.
|
||||
Builder.SetInsertPoint(Preheader->getTerminator());
|
||||
}
|
||||
// Ok, move up a level.
|
||||
Builder.SetInsertPoint(Preheader->getTerminator());
|
||||
}
|
||||
|
||||
// If we haven't found this binop, insert it.
|
||||
|
|
@ -1679,46 +1666,31 @@ Value *SCEVExpander::expand(const SCEV *S) {
|
|||
// Compute an insertion point for this SCEV object. Hoist the instructions
|
||||
// as far out in the loop nest as possible.
|
||||
Instruction *InsertPt = &*Builder.GetInsertPoint();
|
||||
bool SafeToHoist = !SCEVExprContains(S, [](const SCEV *S) {
|
||||
if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {
|
||||
if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))
|
||||
// Division by non-zero constants can be hoisted.
|
||||
return SC->getValue()->isZero();
|
||||
|
||||
// All other divisions should not be moved as they may be divisions by
|
||||
// zero and should be kept within the conditions of the surrounding
|
||||
// loops that guard their execution (see PR30935.)
|
||||
return true;
|
||||
for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;
|
||||
L = L->getParentLoop())
|
||||
if (SE.isLoopInvariant(S, L)) {
|
||||
if (!L) break;
|
||||
if (BasicBlock *Preheader = L->getLoopPreheader())
|
||||
InsertPt = Preheader->getTerminator();
|
||||
else {
|
||||
// LSR sets the insertion point for AddRec start/step values to the
|
||||
// block start to simplify value reuse, even though it's an invalid
|
||||
// position. SCEVExpander must correct for this in all cases.
|
||||
InsertPt = &*L->getHeader()->getFirstInsertionPt();
|
||||
}
|
||||
return false;
|
||||
});
|
||||
if (SafeToHoist) {
|
||||
for (Loop *L = SE.LI.getLoopFor(Builder.GetInsertBlock());;
|
||||
L = L->getParentLoop())
|
||||
if (SE.isLoopInvariant(S, L)) {
|
||||
if (!L) break;
|
||||
if (BasicBlock *Preheader = L->getLoopPreheader())
|
||||
InsertPt = Preheader->getTerminator();
|
||||
else {
|
||||
// LSR sets the insertion point for AddRec start/step values to the
|
||||
// block start to simplify value reuse, even though it's an invalid
|
||||
// position. SCEVExpander must correct for this in all cases.
|
||||
InsertPt = &*L->getHeader()->getFirstInsertionPt();
|
||||
}
|
||||
} else {
|
||||
// If the SCEV is computable at this level, insert it into the header
|
||||
// after the PHIs (and after any other instructions that we've inserted
|
||||
// there) so that it is guaranteed to dominate any user inside the loop.
|
||||
if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L))
|
||||
InsertPt = &*L->getHeader()->getFirstInsertionPt();
|
||||
while (InsertPt->getIterator() != Builder.GetInsertPoint() &&
|
||||
(isInsertedInstruction(InsertPt) ||
|
||||
isa<DbgInfoIntrinsic>(InsertPt))) {
|
||||
InsertPt = &*std::next(InsertPt->getIterator());
|
||||
}
|
||||
break;
|
||||
} else {
|
||||
// If the SCEV is computable at this level, insert it into the header
|
||||
// after the PHIs (and after any other instructions that we've inserted
|
||||
// there) so that it is guaranteed to dominate any user inside the loop.
|
||||
if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L))
|
||||
InsertPt = &*L->getHeader()->getFirstInsertionPt();
|
||||
while (InsertPt->getIterator() != Builder.GetInsertPoint() &&
|
||||
(isInsertedInstruction(InsertPt) ||
|
||||
isa<DbgInfoIntrinsic>(InsertPt))) {
|
||||
InsertPt = &*std::next(InsertPt->getIterator());
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
// Check to see if we already expanded this here.
|
||||
auto I = InsertedExpressions.find(std::make_pair(S, InsertPt));
|
||||
|
|
|
|||
|
|
@ -1,94 +0,0 @@
|
|||
; RUN: opt -loop-idiom -S < %s | FileCheck %s
|
||||
|
||||
; CHECK-LABEL: define i32 @main(
|
||||
; CHECK: udiv
|
||||
; CHECK-NOT: udiv
|
||||
; CHECK: call void @llvm.memset.p0i8.i64
|
||||
|
||||
@a = common local_unnamed_addr global [4 x i8] zeroinitializer, align 1
|
||||
@b = common local_unnamed_addr global i32 0, align 4
|
||||
@c = common local_unnamed_addr global i32 0, align 4
|
||||
@d = common local_unnamed_addr global i32 0, align 4
|
||||
@e = common local_unnamed_addr global i32 0, align 4
|
||||
@f = common local_unnamed_addr global i32 0, align 4
|
||||
@g = common local_unnamed_addr global i32 0, align 4
|
||||
@h = common local_unnamed_addr global i64 0, align 8
|
||||
|
||||
|
||||
define i32 @main() local_unnamed_addr #0 {
|
||||
entry:
|
||||
%0 = load i32, i32* @e, align 4
|
||||
%tobool19 = icmp eq i32 %0, 0
|
||||
%1 = load i32, i32* @c, align 4
|
||||
%cmp10 = icmp eq i32 %1, 0
|
||||
%2 = load i32, i32* @g, align 4
|
||||
%3 = load i32, i32* @b, align 4
|
||||
%tobool = icmp eq i32 %0, 0
|
||||
br label %for.cond
|
||||
|
||||
for.cond.loopexit: ; preds = %for.inc14
|
||||
br label %for.cond.backedge
|
||||
|
||||
for.cond: ; preds = %for.cond.backedge, %entry
|
||||
%.pr = load i32, i32* @f, align 4
|
||||
%cmp20 = icmp eq i32 %.pr, 0
|
||||
br i1 %cmp20, label %for.cond2.preheader.preheader, label %for.cond.backedge
|
||||
|
||||
for.cond.backedge: ; preds = %for.cond, %for.cond.loopexit
|
||||
br label %for.cond
|
||||
|
||||
for.cond2.preheader.preheader: ; preds = %for.cond
|
||||
br label %for.cond2.preheader
|
||||
|
||||
for.cond2.preheader: ; preds = %for.cond2.preheader.preheader, %for.inc14
|
||||
br i1 %tobool19, label %for.cond9, label %for.body3.lr.ph
|
||||
|
||||
for.body3.lr.ph: ; preds = %for.cond2.preheader
|
||||
%div = udiv i32 %2, %3
|
||||
%conv = zext i32 %div to i64
|
||||
br label %for.body3
|
||||
|
||||
for.cond4.for.cond2.loopexit_crit_edge: ; preds = %for.body6
|
||||
store i32 0, i32* @d, align 4
|
||||
br label %for.cond2.loopexit
|
||||
|
||||
for.cond2.loopexit: ; preds = %for.cond4.for.cond2.loopexit_crit_edge, %for.body3
|
||||
br i1 %tobool, label %for.cond2.for.cond9_crit_edge, label %for.body3
|
||||
|
||||
for.body3: ; preds = %for.body3.lr.ph, %for.cond2.loopexit
|
||||
%.pr17 = load i32, i32* @d, align 4
|
||||
%tobool518 = icmp eq i32 %.pr17, 0
|
||||
br i1 %tobool518, label %for.cond2.loopexit, label %for.body6.preheader
|
||||
|
||||
for.body6.preheader: ; preds = %for.body3
|
||||
%4 = zext i32 %.pr17 to i64
|
||||
br label %for.body6
|
||||
|
||||
for.body6: ; preds = %for.body6.preheader, %for.body6
|
||||
%indvars.iv = phi i64 [ %4, %for.body6.preheader ], [ %indvars.iv.next, %for.body6 ]
|
||||
%add = add nuw nsw i64 %conv, %indvars.iv
|
||||
%arrayidx = getelementptr inbounds [4 x i8], [4 x i8]* @a, i64 0, i64 %add
|
||||
store i8 1, i8* %arrayidx, align 1
|
||||
%5 = trunc i64 %indvars.iv to i32
|
||||
%inc = add i32 %5, 1
|
||||
%tobool5 = icmp eq i32 %inc, 0
|
||||
%indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
|
||||
br i1 %tobool5, label %for.cond4.for.cond2.loopexit_crit_edge, label %for.body6
|
||||
|
||||
for.cond2.for.cond9_crit_edge: ; preds = %for.cond2.loopexit
|
||||
store i64 %conv, i64* @h, align 8
|
||||
br label %for.cond9
|
||||
|
||||
for.cond9: ; preds = %for.cond2.for.cond9_crit_edge, %for.cond2.preheader
|
||||
br i1 %cmp10, label %for.body12, label %for.inc14
|
||||
|
||||
for.body12: ; preds = %for.cond9
|
||||
ret i32 0
|
||||
|
||||
for.inc14: ; preds = %for.cond9
|
||||
%6 = load i32, i32* @f, align 4
|
||||
%inc15 = add i32 %6, 1
|
||||
store i32 %inc15, i32* @f, align 4
|
||||
%cmp = icmp eq i32 %inc15, 0
|
||||
br i1 %cmp, label %for.cond2.preheader, label %for.cond.loopexit
|
||||
}
|
||||
|
|
@ -51,21 +51,13 @@ protected:
|
|||
return ScalarEvolution(F, TLI, *AC, *DT, *LI);
|
||||
}
|
||||
|
||||
void runSCEVTest(Module &M, StringRef FuncName,
|
||||
function_ref<void(Function &F, DominatorTree &DT,
|
||||
LoopInfo &LI, ScalarEvolution &SE)>
|
||||
Test) {
|
||||
auto *F = M.getFunction(FuncName);
|
||||
ASSERT_NE(F, nullptr) << "Could not find " << FuncName;
|
||||
ScalarEvolution SE = buildSE(*F);
|
||||
Test(*F, *DT, *LI, SE);
|
||||
}
|
||||
|
||||
void runWithFunctionAndSE(
|
||||
Module &M, StringRef FuncName,
|
||||
function_ref<void(Function &F, ScalarEvolution &SE)> Test) {
|
||||
runSCEVTest(M, FuncName, [&](Function &F, DominatorTree &DT, LoopInfo &LI,
|
||||
ScalarEvolution &SE) { Test(F, SE); });
|
||||
auto *F = M.getFunction(FuncName);
|
||||
ASSERT_NE(F, nullptr) << "Could not find " << FuncName;
|
||||
ScalarEvolution SE = buildSE(*F);
|
||||
Test(*F, SE);
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -357,13 +349,6 @@ static Instruction *getInstructionByName(Function &F, StringRef Name) {
|
|||
llvm_unreachable("Expected to find instruction!");
|
||||
}
|
||||
|
||||
static Argument *getArgByName(Function &F, StringRef Name) {
|
||||
for (auto &A : F.args())
|
||||
if (A.getName() == Name)
|
||||
return &A;
|
||||
llvm_unreachable("Expected to find argument!");
|
||||
}
|
||||
|
||||
TEST_F(ScalarEvolutionsTest, CommutativeExprOperandOrder) {
|
||||
LLVMContext C;
|
||||
SMDiagnostic Err;
|
||||
|
|
@ -547,94 +532,5 @@ TEST_F(ScalarEvolutionsTest, SCEVCompareComplexity) {
|
|||
EXPECT_NE(nullptr, SE.getSCEV(Acc[0]));
|
||||
}
|
||||
|
||||
TEST_F(ScalarEvolutionsTest, BadHoistingSCEVExpander_PR30942) {
|
||||
LLVMContext C;
|
||||
SMDiagnostic Err;
|
||||
std::unique_ptr<Module> M = parseAssemblyString(
|
||||
"target datalayout = \"e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128\" "
|
||||
" "
|
||||
"define void @f_1(i32 %x, i32 %y, i32 %n, i1* %cond_buf) "
|
||||
" local_unnamed_addr { "
|
||||
"entry: "
|
||||
" %entrycond = icmp sgt i32 %n, 0 "
|
||||
" br i1 %entrycond, label %loop.ph, label %for.end "
|
||||
" "
|
||||
"loop.ph: "
|
||||
" br label %loop "
|
||||
" "
|
||||
"loop: "
|
||||
" %iv1 = phi i32 [ %iv1.inc, %right ], [ 0, %loop.ph ] "
|
||||
" %iv1.inc = add nuw nsw i32 %iv1, 1 "
|
||||
" %cond = load volatile i1, i1* %cond_buf "
|
||||
" br i1 %cond, label %left, label %right "
|
||||
" "
|
||||
"left: "
|
||||
" %div = udiv i32 %x, %y "
|
||||
" br label %right "
|
||||
" "
|
||||
"right: "
|
||||
" %exitcond = icmp eq i32 %iv1.inc, %n "
|
||||
" br i1 %exitcond, label %for.end.loopexit, label %loop "
|
||||
" "
|
||||
"for.end.loopexit: "
|
||||
" br label %for.end "
|
||||
" "
|
||||
"for.end: "
|
||||
" ret void "
|
||||
"} ",
|
||||
Err, C);
|
||||
|
||||
assert(M && "Could not parse module?");
|
||||
assert(!verifyModule(*M) && "Must have been well formed!");
|
||||
|
||||
runSCEVTest(*M, "f_1", [&](Function &F, DominatorTree &DT, LoopInfo &LI,
|
||||
ScalarEvolution &SE) {
|
||||
SCEVExpander Expander(SE, M->getDataLayout(), "unittests");
|
||||
auto *DivInst = getInstructionByName(F, "div");
|
||||
|
||||
{
|
||||
auto *DivSCEV = SE.getSCEV(DivInst);
|
||||
auto *DivExpansion = Expander.expandCodeFor(
|
||||
DivSCEV, DivSCEV->getType(), DivInst->getParent()->getTerminator());
|
||||
auto *DivExpansionInst = dyn_cast<Instruction>(DivExpansion);
|
||||
ASSERT_NE(DivExpansionInst, nullptr);
|
||||
EXPECT_EQ(DivInst->getParent(), DivExpansionInst->getParent());
|
||||
}
|
||||
|
||||
{
|
||||
auto *ArgY = getArgByName(F, "y");
|
||||
auto *DivFromScratchSCEV =
|
||||
SE.getUDivExpr(SE.getOne(ArgY->getType()), SE.getSCEV(ArgY));
|
||||
|
||||
auto *DivFromScratchExpansion = Expander.expandCodeFor(
|
||||
DivFromScratchSCEV, DivFromScratchSCEV->getType(),
|
||||
DivInst->getParent()->getTerminator());
|
||||
auto *DivFromScratchExpansionInst =
|
||||
dyn_cast<Instruction>(DivFromScratchExpansion);
|
||||
ASSERT_NE(DivFromScratchExpansionInst, nullptr);
|
||||
EXPECT_EQ(DivInst->getParent(), DivFromScratchExpansionInst->getParent());
|
||||
}
|
||||
|
||||
{
|
||||
auto *ArgY = getArgByName(F, "y");
|
||||
auto *One = SE.getOne(ArgY->getType());
|
||||
auto *DivFromScratchSCEV = SE.getUDivExpr(One, SE.getSCEV(ArgY));
|
||||
auto *L = LI.getLoopFor(DivInst->getParent());
|
||||
auto *ARFromScratchSCEV =
|
||||
SE.getAddRecExpr(DivFromScratchSCEV, One, L, SCEV::FlagAnyWrap);
|
||||
|
||||
Expander.disableCanonicalMode();
|
||||
|
||||
auto *ARFromScratchExpansion = Expander.expandCodeFor(
|
||||
ARFromScratchSCEV, ARFromScratchSCEV->getType(),
|
||||
DivInst->getParent()->getTerminator());
|
||||
auto *ARFromScratchExpansionInst =
|
||||
dyn_cast<Instruction>(ARFromScratchExpansion);
|
||||
ASSERT_NE(ARFromScratchExpansionInst, nullptr);
|
||||
ASSERT_FALSE(verifyFunction(F));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
} // end anonymous namespace
|
||||
} // end namespace llvm
|
||||
|
|
|
|||
Loading…
Reference in New Issue