Skip to content

Commit 74854d0

Browse files
fhahnfhahn
committed
Revert "[NewGVN] Track simplification dependencies for phi-of-ops."
This reverts commit 4f5da35. This causes some buildbot failures, e.g. https://lab.llvm.org/buildbot/#/builders/139/builds/3019
1 parent f6a3e92 commit 74854d0

File tree

2 files changed

+69
-209
lines changed

2 files changed

+69
-209
lines changed

‎llvm/lib/Transforms/Scalar/NewGVN.cpp

Lines changed: 69 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -668,23 +668,8 @@ class NewGVN {
668668
bool runGVN();
669669

670670
private:
671-
/// Helper struct return a Expression with an optional extra dependency.
672-
struct ExprResult {
673-
const Expression *Expr;
674-
Value *ExtraDep;
675-
676-
~ExprResult() { assert(!ExtraDep && "unhandled ExtraDep"); }
677-
678-
operator bool() const { return Expr; }
679-
680-
static ExprResult none() { return {nullptr, nullptr}; }
681-
static ExprResult some(const Expression *Expr, Value *ExtraDep = nullptr) {
682-
return {Expr, ExtraDep};
683-
}
684-
};
685-
686671
// Expression handling.
687-
ExprResult createExpression(Instruction *) const;
672+
const Expression *createExpression(Instruction *) const;
688673
const Expression *createBinaryExpression(unsigned, Type *, Value *, Value *,
689674
Instruction *) const;
690675

@@ -757,9 +742,10 @@ class NewGVN {
757742
void valueNumberInstruction(Instruction *);
758743

759744
// Symbolic evaluation.
760-
ExprResult checkExprResults(Expression *, Instruction *, Value *) const;
761-
ExprResult performSymbolicEvaluation(Value *,
762-
SmallPtrSetImpl<Value *> &) const;
745+
const Expression *checkSimplificationResults(Expression *, Instruction *,
746+
Value *) const;
747+
const Expression *performSymbolicEvaluation(Value *,
748+
SmallPtrSetImpl<Value *> &) const;
763749
const Expression *performSymbolicLoadCoercion(Type *, Value *, LoadInst *,
764750
Instruction *,
765751
MemoryAccess *) const;
@@ -771,7 +757,7 @@ class NewGVN {
771757
Instruction *I,
772758
BasicBlock *PHIBlock) const;
773759
const Expression *performSymbolicAggrValueEvaluation(Instruction *) const;
774-
ExprResult performSymbolicCmpEvaluation(Instruction *) const;
760+
const Expression *performSymbolicCmpEvaluation(Instruction *) const;
775761
const Expression *performSymbolicPredicateInfoEvaluation(Instruction *) const;
776762

777763
// Congruence finding.
@@ -828,7 +814,6 @@ class NewGVN {
828814
void addPredicateUsers(const PredicateBase *, Instruction *) const;
829815
void addMemoryUsers(const MemoryAccess *To, MemoryAccess *U) const;
830816
void addAdditionalUsers(Value *To, Value *User) const;
831-
void addAdditionalUsers(ExprResult &Res, Value *User) const;
832817

833818
// Main loop of value numbering
834819
void iterateTouchedInstructions();
@@ -1067,21 +1052,19 @@ const Expression *NewGVN::createBinaryExpression(unsigned Opcode, Type *T,
10671052
E->op_push_back(lookupOperandLeader(Arg2));
10681053

10691054
Value *V = SimplifyBinOp(Opcode, E->getOperand(0), E->getOperand(1), SQ);
1070-
if (auto Simplified = checkExprResults(E, I, V)) {
1071-
addAdditionalUsers(Simplified, I);
1072-
return Simplified.Expr;
1073-
}
1055+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1056+
return SimplifiedE;
10741057
return E;
10751058
}
10761059

10771060
// Take a Value returned by simplification of Expression E/Instruction
10781061
// I, and see if it resulted in a simpler expression. If so, return
10791062
// that expression.
1080-
NewGVN::ExprResult NewGVN::checkExprResults(Expression *E, Instruction *I,
1081-
Value *V) const {
1063+
const Expression *NewGVN::checkSimplificationResults(Expression *E,
1064+
Instruction *I,
1065+
Value *V) const {
10821066
if (!V)
1083-
return ExprResult::none();
1084-
1067+
return nullptr;
10851068
if (auto *C = dyn_cast<Constant>(V)) {
10861069
if (I)
10871070
LLVM_DEBUG(dbgs() << "Simplified " << *I << " to "
@@ -1090,37 +1073,52 @@ NewGVN::ExprResult NewGVN::checkExprResults(Expression *E, Instruction *I,
10901073
assert(isa<BasicExpression>(E) &&
10911074
"We should always have had a basic expression here");
10921075
deleteExpression(E);
1093-
return ExprResult::some(createConstantExpression(C));
1076+
return createConstantExpression(C);
10941077
} else if (isa<Argument>(V) || isa<GlobalVariable>(V)) {
10951078
if (I)
10961079
LLVM_DEBUG(dbgs() << "Simplified " << *I << " to "
10971080
<< " variable " << *V << "\n");
10981081
deleteExpression(E);
1099-
return ExprResult::some(createVariableExpression(V));
1082+
return createVariableExpression(V);
11001083
}
11011084

11021085
CongruenceClass *CC = ValueToClass.lookup(V);
11031086
if (CC) {
11041087
if (CC->getLeader() && CC->getLeader() != I) {
1105-
return ExprResult::some(createVariableOrConstant(CC->getLeader()), V);
1088+
// If we simplified to something else, we need to communicate
1089+
// that we're users of the value we simplified to.
1090+
if (I != V) {
1091+
// Don't add temporary instructions to the user lists.
1092+
if (!AllTempInstructions.count(I))
1093+
addAdditionalUsers(V, I);
1094+
}
1095+
return createVariableOrConstant(CC->getLeader());
11061096
}
11071097
if (CC->getDefiningExpr()) {
1098+
// If we simplified to something else, we need to communicate
1099+
// that we're users of the value we simplified to.
1100+
if (I != V) {
1101+
// Don't add temporary instructions to the user lists.
1102+
if (!AllTempInstructions.count(I))
1103+
addAdditionalUsers(V, I);
1104+
}
1105+
11081106
if (I)
11091107
LLVM_DEBUG(dbgs() << "Simplified " << *I << " to "
11101108
<< " expression " << *CC->getDefiningExpr() << "\n");
11111109
NumGVNOpsSimplified++;
11121110
deleteExpression(E);
1113-
return ExprResult::some(CC->getDefiningExpr(), V);
1111+
return CC->getDefiningExpr();
11141112
}
11151113
}
11161114

1117-
return ExprResult::none();
1115+
return nullptr;
11181116
}
11191117

11201118
// Create a value expression from the instruction I, replacing operands with
11211119
// their leaders.
11221120

1123-
NewGVN::ExprResult NewGVN::createExpression(Instruction *I) const {
1121+
const Expression *NewGVN::createExpression(Instruction *I) const {
11241122
auto *E = new (ExpressionAllocator) BasicExpression(I->getNumOperands());
11251123

11261124
bool AllConstant = setBasicExpressionInfo(I, E);
@@ -1151,33 +1149,33 @@ NewGVN::ExprResult NewGVN::createExpression(Instruction *I) const {
11511149
E->getOperand(1)->getType() == I->getOperand(1)->getType()));
11521150
Value *V =
11531151
SimplifyCmpInst(Predicate, E->getOperand(0), E->getOperand(1), SQ);
1154-
if (auto Simplified = checkExprResults(E, I, V))
1155-
return Simplified;
1152+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1153+
return SimplifiedE;
11561154
} else if (isa<SelectInst>(I)) {
11571155
if (isa<Constant>(E->getOperand(0)) ||
11581156
E->getOperand(1) == E->getOperand(2)) {
11591157
assert(E->getOperand(1)->getType() == I->getOperand(1)->getType() &&
11601158
E->getOperand(2)->getType() == I->getOperand(2)->getType());
11611159
Value *V = SimplifySelectInst(E->getOperand(0), E->getOperand(1),
11621160
E->getOperand(2), SQ);
1163-
if (auto Simplified = checkExprResults(E, I, V))
1164-
return Simplified;
1161+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1162+
return SimplifiedE;
11651163
}
11661164
} else if (I->isBinaryOp()) {
11671165
Value *V =
11681166
SimplifyBinOp(E->getOpcode(), E->getOperand(0), E->getOperand(1), SQ);
1169-
if (auto Simplified = checkExprResults(E, I, V))
1170-
return Simplified;
1167+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1168+
return SimplifiedE;
11711169
} else if (auto *CI = dyn_cast<CastInst>(I)) {
11721170
Value *V =
11731171
SimplifyCastInst(CI->getOpcode(), E->getOperand(0), CI->getType(), SQ);
1174-
if (auto Simplified = checkExprResults(E, I, V))
1175-
return Simplified;
1172+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1173+
return SimplifiedE;
11761174
} else if (isa<GetElementPtrInst>(I)) {
11771175
Value *V = SimplifyGEPInst(
11781176
E->getType(), ArrayRef<Value *>(E->op_begin(), E->op_end()), SQ);
1179-
if (auto Simplified = checkExprResults(E, I, V))
1180-
return Simplified;
1177+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1178+
return SimplifiedE;
11811179
} else if (AllConstant) {
11821180
// We don't bother trying to simplify unless all of the operands
11831181
// were constant.
@@ -1191,10 +1189,10 @@ NewGVN::ExprResult NewGVN::createExpression(Instruction *I) const {
11911189
C.emplace_back(cast<Constant>(Arg));
11921190

11931191
if (Value *V = ConstantFoldInstOperands(I, C, DL, TLI))
1194-
if (auto Simplified = checkExprResults(E, I, V))
1195-
return Simplified;
1192+
if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
1193+
return SimplifiedE;
11961194
}
1197-
return ExprResult::some(E);
1195+
return E;
11981196
}
11991197

12001198
const AggregateValueExpression *
@@ -1780,7 +1778,7 @@ NewGVN::performSymbolicAggrValueEvaluation(Instruction *I) const {
17801778
return createAggregateValueExpression(I);
17811779
}
17821780

1783-
NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
1781+
const Expression *NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
17841782
assert(isa<CmpInst>(I) && "Expected a cmp instruction.");
17851783

17861784
auto *CI = cast<CmpInst>(I);
@@ -1800,17 +1798,14 @@ NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
18001798
// of an assume.
18011799
auto *CmpPI = PredInfo->getPredicateInfoFor(I);
18021800
if (dyn_cast_or_null<PredicateAssume>(CmpPI))
1803-
return ExprResult::some(
1804-
createConstantExpression(ConstantInt::getTrue(CI->getType())));
1801+
return createConstantExpression(ConstantInt::getTrue(CI->getType()));
18051802

18061803
if (Op0 == Op1) {
18071804
// This condition does not depend on predicates, no need to add users
18081805
if (CI->isTrueWhenEqual())
1809-
return ExprResult::some(
1810-
createConstantExpression(ConstantInt::getTrue(CI->getType())));
1806+
return createConstantExpression(ConstantInt::getTrue(CI->getType()));
18111807
else if (CI->isFalseWhenEqual())
1812-
return ExprResult::some(
1813-
createConstantExpression(ConstantInt::getFalse(CI->getType())));
1808+
return createConstantExpression(ConstantInt::getFalse(CI->getType()));
18141809
}
18151810

18161811
// NOTE: Because we are comparing both operands here and below, and using
@@ -1870,30 +1865,30 @@ NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
18701865
if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate,
18711866
OurPredicate)) {
18721867
addPredicateUsers(PI, I);
1873-
return ExprResult::some(
1874-
createConstantExpression(ConstantInt::getTrue(CI->getType())));
1868+
return createConstantExpression(
1869+
ConstantInt::getTrue(CI->getType()));
18751870
}
18761871

18771872
if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate,
18781873
OurPredicate)) {
18791874
addPredicateUsers(PI, I);
1880-
return ExprResult::some(
1881-
createConstantExpression(ConstantInt::getFalse(CI->getType())));
1875+
return createConstantExpression(
1876+
ConstantInt::getFalse(CI->getType()));
18821877
}
18831878
} else {
18841879
// Just handle the ne and eq cases, where if we have the same
18851880
// operands, we may know something.
18861881
if (BranchPredicate == OurPredicate) {
18871882
addPredicateUsers(PI, I);
18881883
// Same predicate, same ops,we know it was false, so this is false.
1889-
return ExprResult::some(
1890-
createConstantExpression(ConstantInt::getFalse(CI->getType())));
1884+
return createConstantExpression(
1885+
ConstantInt::getFalse(CI->getType()));
18911886
} else if (BranchPredicate ==
18921887
CmpInst::getInversePredicate(OurPredicate)) {
18931888
addPredicateUsers(PI, I);
18941889
// Inverse predicate, we know the other was false, so this is true.
1895-
return ExprResult::some(
1896-
createConstantExpression(ConstantInt::getTrue(CI->getType())));
1890+
return createConstantExpression(
1891+
ConstantInt::getTrue(CI->getType()));
18971892
}
18981893
}
18991894
}
@@ -1904,10 +1899,9 @@ NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
19041899
}
19051900

19061901
// Substitute and symbolize the value before value numbering.
1907-
NewGVN::ExprResult
1902+
const Expression *
19081903
NewGVN::performSymbolicEvaluation(Value *V,
19091904
SmallPtrSetImpl<Value *> &Visited) const {
1910-
19111905
const Expression *E = nullptr;
19121906
if (auto *C = dyn_cast<Constant>(V))
19131907
E = createConstantExpression(C);
@@ -1943,11 +1937,11 @@ NewGVN::performSymbolicEvaluation(Value *V,
19431937
break;
19441938
case Instruction::BitCast:
19451939
case Instruction::AddrSpaceCast:
1946-
return createExpression(I);
1940+
E = createExpression(I);
19471941
break;
19481942
case Instruction::ICmp:
19491943
case Instruction::FCmp:
1950-
return performSymbolicCmpEvaluation(I);
1944+
E = performSymbolicCmpEvaluation(I);
19511945
break;
19521946
case Instruction::FNeg:
19531947
case Instruction::Add:
@@ -1983,16 +1977,16 @@ NewGVN::performSymbolicEvaluation(Value *V,
19831977
case Instruction::ExtractElement:
19841978
case Instruction::InsertElement:
19851979
case Instruction::GetElementPtr:
1986-
return createExpression(I);
1980+
E = createExpression(I);
19871981
break;
19881982
case Instruction::ShuffleVector:
19891983
// FIXME: Add support for shufflevector to createExpression.
1990-
return ExprResult::none();
1984+
return nullptr;
19911985
default:
1992-
return ExprResult::none();
1986+
return nullptr;
19931987
}
19941988
}
1995-
return ExprResult::some(E);
1989+
return E;
19961990
}
19971991

19981992
// Look up a container of values/instructions in a map, and touch all the
@@ -2013,12 +2007,6 @@ void NewGVN::addAdditionalUsers(Value *To, Value *User) const {
20132007
AdditionalUsers[To].insert(User);
20142008
}
20152009

2016-
void NewGVN::addAdditionalUsers(ExprResult &Res, Value *User) const {
2017-
if (Res.ExtraDep && Res.ExtraDep != User)
2018-
addAdditionalUsers(Res.ExtraDep, User);
2019-
Res.ExtraDep = nullptr;
2020-
}
2021-
20222010
void NewGVN::markUsersTouched(Value *V) {
20232011
// Now mark the users as touched.
20242012
for (auto *User : V->users()) {
@@ -2426,14 +2414,9 @@ void NewGVN::processOutgoingEdges(Instruction *TI, BasicBlock *B) {
24262414
Value *CondEvaluated = findConditionEquivalence(Cond);
24272415
if (!CondEvaluated) {
24282416
if (auto *I = dyn_cast<Instruction>(Cond)) {
2429-
auto Res = createExpression(I);
2430-
if (const auto *CE = dyn_cast<ConstantExpression>(Res.Expr)) {
2417+
const Expression *E = createExpression(I);
2418+
if (const auto *CE = dyn_cast<ConstantExpression>(E)) {
24312419
CondEvaluated = CE->getConstantValue();
2432-
addAdditionalUsers(Res, I);
2433-
} else {
2434-
// Did not use simplification result, no need to add the extra
2435-
// dependency.
2436-
Res.ExtraDep = nullptr;
24372420
}
24382421
} else if (isa<ConstantInt>(Cond)) {
24392422
CondEvaluated = Cond;
@@ -2617,9 +2600,7 @@ Value *NewGVN::findLeaderForInst(Instruction *TransInst,
26172600
TempToBlock.insert({TransInst, PredBB});
26182601
InstrDFS.insert({TransInst, IDFSNum});
26192602

2620-
auto Res = performSymbolicEvaluation(TransInst, Visited);
2621-
const Expression *E = Res.Expr;
2622-
addAdditionalUsers(Res, OrigInst);
2603+
const Expression *E = performSymbolicEvaluation(TransInst, Visited);
26232604
InstrDFS.erase(TransInst);
26242605
AllTempInstructions.erase(TransInst);
26252606
TempToBlock.erase(TransInst);
@@ -3046,10 +3027,7 @@ void NewGVN::valueNumberInstruction(Instruction *I) {
30463027
const Expression *Symbolized = nullptr;
30473028
SmallPtrSet<Value *, 2> Visited;
30483029
if (DebugCounter::shouldExecute(VNCounter)) {
3049-
auto Res = performSymbolicEvaluation(I, Visited);
3050-
Symbolized = Res.Expr;
3051-
addAdditionalUsers(Res, I);
3052-
3030+
Symbolized = performSymbolicEvaluation(I, Visited);
30533031
// Make a phi of ops if necessary
30543032
if (Symbolized && !isa<ConstantExpression>(Symbolized) &&
30553033
!isa<VariableExpression>(Symbolized) && PHINodeUses.count(I)) {

0 commit comments

Comments
 (0)