[llvm] r321475 - [SCEV] Be careful with nuw/nsw/exact in InsertBinop
Serguei Katkov via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 27 00:26:22 PST 2017
Author: skatkov
Date: Wed Dec 27 00:26:22 2017
New Revision: 321475
URL: http://llvm.org/viewvc/llvm-project?rev=321475&view=rev
Log:
[SCEV] Be careful with nuw/nsw/exact in InsertBinop
InsertBinop tries to find an appropriate instruction instead of
creating a new instruction. When it checks whether instruction is
the same as we need to create it ignores nuw/nsw/exact flags.
It leads to invalid behavior when poison instruction can be used
when it was not expected. Specifically, for example Expander
expands the SCEV built for instruction
%a = add i32 %v, 1
It is possible that InsertBinop can find an instruction
% b = add nuw nsw i32 %v, 1
and will use it instead of version w/o nuw nsw.
It is incorrect.
The patch conservatively ignores all instructions with any of
poison flags installed.
Reviewers: sanjoy, mkazantsev, sebpop, jbhateja
Reviewed By: sanjoy
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D41576
Modified:
llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=321475&r1=321474&r2=321475&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Wed Dec 27 00:26:22 2017
@@ -187,8 +187,21 @@ Value *SCEVExpander::InsertBinop(Instruc
// generated code.
if (isa<DbgInfoIntrinsic>(IP))
ScanLimit++;
+
+ // Conservatively, do not use any instruction which has any of wrap/exact
+ // flags installed.
+ // TODO: Instead of simply disable poison instructions we can be clever
+ // here and match SCEV to this instruction.
+ auto canGeneratePoison = [](Instruction *I) {
+ if (isa<OverflowingBinaryOperator>(I) &&
+ (I->hasNoSignedWrap() || I->hasNoUnsignedWrap()))
+ return true;
+ if (isa<PossiblyExactOperator>(I) && I->isExact())
+ return true;
+ return false;
+ };
if (IP->getOpcode() == (unsigned)Opcode && IP->getOperand(0) == LHS &&
- IP->getOperand(1) == RHS)
+ IP->getOperand(1) == RHS && !canGeneratePoison(&*IP))
return &*IP;
if (IP == BlockBegin) break;
}
Modified: llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp?rev=321475&r1=321474&r2=321475&view=diff
==============================================================================
--- llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp (original)
+++ llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp Wed Dec 27 00:26:22 2017
@@ -1184,5 +1184,109 @@ TEST_F(ScalarEvolutionsTest, SCEVExpande
EXPECT_TRUE(isSafeToExpandAt(AR, Post->getTerminator(), SE));
}
+// Check that SCEV expander does not use the nuw instruction
+// for expansion.
+TEST_F(ScalarEvolutionsTest, SCEVExpanderNUW) {
+ /*
+ * Create the following code:
+ * func(i64 %a)
+ * entry:
+ * br false, label %exit, label %body
+ * body:
+ * %s1 = add i64 %a, -1
+ * br label %exit
+ * exit:
+ * %s = add nuw i64 %a, -1
+ * ret %s
+ */
+
+ // Create a module.
+ Module M("SCEVExpanderNUW", Context);
+
+ Type *T_int64 = Type::getInt64Ty(Context);
+
+ FunctionType *FTy =
+ FunctionType::get(Type::getVoidTy(Context), { T_int64 }, false);
+ Function *F = cast<Function>(M.getOrInsertFunction("func", FTy));
+ Argument *Arg = &*F->arg_begin();
+ ConstantInt *C = ConstantInt::get(Context, APInt(64, -1));
+
+ BasicBlock *Entry = BasicBlock::Create(Context, "entry", F);
+ BasicBlock *Body = BasicBlock::Create(Context, "body", F);
+ BasicBlock *Exit = BasicBlock::Create(Context, "exit", F);
+
+ IRBuilder<> Builder(Entry);
+ ConstantInt *Cond = ConstantInt::get(Context, APInt(1, 0));
+ Builder.CreateCondBr(Cond, Exit, Body);
+
+ Builder.SetInsertPoint(Body);
+ auto *S1 = cast<Instruction>(Builder.CreateAdd(Arg, C, "add"));
+ Builder.CreateBr(Exit);
+
+ Builder.SetInsertPoint(Exit);
+ auto *S2 = cast<Instruction>(Builder.CreateAdd(Arg, C, "add"));
+ S2->setHasNoUnsignedWrap(true);
+ auto *R = cast<Instruction>(Builder.CreateRetVoid());
+
+ ScalarEvolution SE = buildSE(*F);
+ const SCEV *S = SE.getSCEV(S1);
+ EXPECT_TRUE(isa<SCEVAddExpr>(S));
+ SCEVExpander Exp(SE, M.getDataLayout(), "expander");
+ auto *I = cast<Instruction>(Exp.expandCodeFor(S, nullptr, R));
+ EXPECT_FALSE(I->hasNoUnsignedWrap());
+}
+
+// Check that SCEV expander does not use the nsw instruction
+// for expansion.
+TEST_F(ScalarEvolutionsTest, SCEVExpanderNSW) {
+ /*
+ * Create the following code:
+ * func(i64 %a)
+ * entry:
+ * br false, label %exit, label %body
+ * body:
+ * %s1 = add i64 %a, -1
+ * br label %exit
+ * exit:
+ * %s = add nsw i64 %a, -1
+ * ret %s
+ */
+
+ // Create a module.
+ Module M("SCEVExpanderNSW", Context);
+
+ Type *T_int64 = Type::getInt64Ty(Context);
+
+ FunctionType *FTy =
+ FunctionType::get(Type::getVoidTy(Context), { T_int64 }, false);
+ Function *F = cast<Function>(M.getOrInsertFunction("func", FTy));
+ Argument *Arg = &*F->arg_begin();
+ ConstantInt *C = ConstantInt::get(Context, APInt(64, -1));
+
+ BasicBlock *Entry = BasicBlock::Create(Context, "entry", F);
+ BasicBlock *Body = BasicBlock::Create(Context, "body", F);
+ BasicBlock *Exit = BasicBlock::Create(Context, "exit", F);
+
+ IRBuilder<> Builder(Entry);
+ ConstantInt *Cond = ConstantInt::get(Context, APInt(1, 0));
+ Builder.CreateCondBr(Cond, Exit, Body);
+
+ Builder.SetInsertPoint(Body);
+ auto *S1 = cast<Instruction>(Builder.CreateAdd(Arg, C, "add"));
+ Builder.CreateBr(Exit);
+
+ Builder.SetInsertPoint(Exit);
+ auto *S2 = cast<Instruction>(Builder.CreateAdd(Arg, C, "add"));
+ S2->setHasNoSignedWrap(true);
+ auto *R = cast<Instruction>(Builder.CreateRetVoid());
+
+ ScalarEvolution SE = buildSE(*F);
+ const SCEV *S = SE.getSCEV(S1);
+ EXPECT_TRUE(isa<SCEVAddExpr>(S));
+ SCEVExpander Exp(SE, M.getDataLayout(), "expander");
+ auto *I = cast<Instruction>(Exp.expandCodeFor(S, nullptr, R));
+ EXPECT_FALSE(I->hasNoSignedWrap());
+}
+
} // end anonymous namespace
} // end namespace llvm
More information about the llvm-commits
mailing list