Skip to content

Commit 01c5ec3

Browse files
mtrofinmtrofin
committed
[NFC] Surface the validation of FunctionPropertiesAnalysis
Avoids relying on `assert` for some of the validation. Differential Revision: https://reviews.llvm.org/D150827
1 parent 590eb76 commit 01c5ec3

File tree

3 files changed

+44
-22
lines changed

3 files changed

+44
-22
lines changed

‎llvm/include/llvm/Analysis/FunctionPropertiesAnalysis.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "llvm/IR/PassManager.h"
2121

2222
namespace llvm {
23+
class DominatorTree;
2324
class Function;
2425
class LoopInfo;
2526

@@ -31,7 +32,11 @@ class FunctionPropertiesInfo {
3132

3233
public:
3334
static FunctionPropertiesInfo
34-
getFunctionPropertiesInfo(const Function &F, FunctionAnalysisManager &FAM);
35+
getFunctionPropertiesInfo(const Function &F, const DominatorTree &DT,
36+
const LoopInfo &LI);
37+
38+
static FunctionPropertiesInfo
39+
getFunctionPropertiesInfo(Function &F, FunctionAnalysisManager &FAM);
3540

3641
bool operator==(const FunctionPropertiesInfo &FPI) const {
3742
return std::memcmp(this, &FPI, sizeof(FunctionPropertiesInfo)) == 0;
@@ -109,14 +114,21 @@ class FunctionPropertiesPrinterPass
109114
/// inlining.
110115
class FunctionPropertiesUpdater {
111116
public:
112-
FunctionPropertiesUpdater(FunctionPropertiesInfo &FPI, const CallBase &CB);
117+
FunctionPropertiesUpdater(FunctionPropertiesInfo &FPI, CallBase &CB);
113118

114119
void finish(FunctionAnalysisManager &FAM) const;
120+
bool finishAndTest(FunctionAnalysisManager &FAM) const {
121+
finish(FAM);
122+
return isUpdateValid(Caller, FPI, FAM);
123+
}
115124

116125
private:
117126
FunctionPropertiesInfo &FPI;
118-
const BasicBlock &CallSiteBB;
119-
const Function &Caller;
127+
BasicBlock &CallSiteBB;
128+
Function &Caller;
129+
130+
static bool isUpdateValid(Function &F, const FunctionPropertiesInfo &FPI,
131+
FunctionAnalysisManager &FAM);
120132

121133
DenseSet<const BasicBlock *> Successors;
122134
};

‎llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,15 @@ void FunctionPropertiesInfo::updateAggregateStats(const Function &F,
8282
}
8383

8484
FunctionPropertiesInfo FunctionPropertiesInfo::getFunctionPropertiesInfo(
85-
const Function &F, FunctionAnalysisManager &FAM) {
85+
Function &F, FunctionAnalysisManager &FAM) {
86+
return getFunctionPropertiesInfo(F, FAM.getResult<DominatorTreeAnalysis>(F),
87+
FAM.getResult<LoopAnalysis>(F));
88+
}
89+
90+
FunctionPropertiesInfo FunctionPropertiesInfo::getFunctionPropertiesInfo(
91+
const Function &F, const DominatorTree &DT, const LoopInfo &LI) {
8692

8793
FunctionPropertiesInfo FPI;
88-
// The const casts are due to the getResult API - there's no mutation of F.
89-
const auto &LI = FAM.getResult<LoopAnalysis>(const_cast<Function &>(F));
90-
const auto &DT =
91-
FAM.getResult<DominatorTreeAnalysis>(const_cast<Function &>(F));
9294
for (const auto &BB : F)
9395
if (DT.isReachableFromEntry(&BB))
9496
FPI.reIncludeBB(BB);
@@ -127,7 +129,7 @@ FunctionPropertiesPrinterPass::run(Function &F, FunctionAnalysisManager &AM) {
127129
}
128130

129131
FunctionPropertiesUpdater::FunctionPropertiesUpdater(
130-
FunctionPropertiesInfo &FPI, const CallBase &CB)
132+
FunctionPropertiesInfo &FPI, CallBase &CB)
131133
: FPI(FPI), CallSiteBB(*CB.getParent()), Caller(*CallSiteBB.getParent()) {
132134
assert(isa<CallInst>(CB) || isa<InvokeInst>(CB));
133135
// For BBs that are likely to change, we subtract from feature totals their
@@ -247,5 +249,13 @@ void FunctionPropertiesUpdater::finish(FunctionAnalysisManager &FAM) const {
247249

248250
const auto &LI = FAM.getResult<LoopAnalysis>(const_cast<Function &>(Caller));
249251
FPI.updateAggregateStats(Caller, LI);
250-
assert(FPI == FunctionPropertiesInfo::getFunctionPropertiesInfo(Caller, FAM));
251252
}
253+
254+
bool FunctionPropertiesUpdater::isUpdateValid(Function &F,
255+
const FunctionPropertiesInfo &FPI,
256+
FunctionAnalysisManager &FAM) {
257+
DominatorTree DT(F);
258+
LoopInfo LI(DT);
259+
auto Fresh = FunctionPropertiesInfo::getFunctionPropertiesInfo(F, DT, LI);
260+
return FPI == Fresh;
261+
}

‎llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ define i32 @f2(i32 %a) {
158158
auto IR = llvm::InlineFunction(*CB, IFI);
159159
EXPECT_TRUE(IR.isSuccess());
160160
invalidate(*F1);
161-
FPU.finish(FAM);
161+
EXPECT_TRUE(FPU.finishAndTest(FAM));
162162
EXPECT_EQ(FPI, ExpectedFinal);
163163
}
164164

@@ -212,7 +212,7 @@ define i32 @f2(i32 %a) {
212212
auto IR = llvm::InlineFunction(*CB, IFI);
213213
EXPECT_TRUE(IR.isSuccess());
214214
invalidate(*F1);
215-
FPU.finish(FAM);
215+
EXPECT_TRUE(FPU.finishAndTest(FAM));
216216
EXPECT_EQ(FPI, ExpectedFinal);
217217
}
218218

@@ -279,7 +279,7 @@ define i32 @f2(i32 %a) {
279279
auto IR = llvm::InlineFunction(*CB, IFI);
280280
EXPECT_TRUE(IR.isSuccess());
281281
invalidate(*F1);
282-
FPU.finish(FAM);
282+
EXPECT_TRUE(FPU.finishAndTest(FAM));
283283
EXPECT_EQ(FPI, ExpectedFinal);
284284
}
285285

@@ -324,7 +324,7 @@ declare i32 @__gxx_personality_v0(...)
324324
auto IR = llvm::InlineFunction(*CB, IFI);
325325
EXPECT_TRUE(IR.isSuccess());
326326
invalidate(*F1);
327-
FPU.finish(FAM);
327+
EXPECT_TRUE(FPU.finishAndTest(FAM));
328328
EXPECT_EQ(static_cast<size_t>(FPI.BasicBlockCount), F1->size());
329329
EXPECT_EQ(static_cast<size_t>(FPI.TotalInstructionCount),
330330
F1->getInstructionCount());
@@ -377,7 +377,7 @@ declare i32 @__gxx_personality_v0(...)
377377
auto IR = llvm::InlineFunction(*CB, IFI);
378378
EXPECT_TRUE(IR.isSuccess());
379379
invalidate(*F1);
380-
FPU.finish(FAM);
380+
EXPECT_TRUE(FPU.finishAndTest(FAM));
381381
EXPECT_EQ(static_cast<size_t>(FPI.BasicBlockCount), F1->size() - 1);
382382
EXPECT_EQ(static_cast<size_t>(FPI.TotalInstructionCount),
383383
F1->getInstructionCount() - 2);
@@ -431,7 +431,7 @@ declare i32 @__gxx_personality_v0(...)
431431
auto IR = llvm::InlineFunction(*CB, IFI);
432432
EXPECT_TRUE(IR.isSuccess());
433433
invalidate(*F1);
434-
FPU.finish(FAM);
434+
EXPECT_TRUE(FPU.finishAndTest(FAM));
435435
EXPECT_EQ(static_cast<size_t>(FPI.BasicBlockCount), F1->size() - 1);
436436
EXPECT_EQ(static_cast<size_t>(FPI.TotalInstructionCount),
437437
F1->getInstructionCount() - 2);
@@ -483,7 +483,7 @@ define void @outer() personality i8* null {
483483
auto IR = llvm::InlineFunction(*CB, IFI);
484484
EXPECT_TRUE(IR.isSuccess());
485485
invalidate(*F1);
486-
FPU.finish(FAM);
486+
EXPECT_TRUE(FPU.finishAndTest(FAM));
487487
EXPECT_EQ(static_cast<size_t>(FPI.BasicBlockCount), F1->size() - 1);
488488
EXPECT_EQ(static_cast<size_t>(FPI.TotalInstructionCount),
489489
F1->getInstructionCount() - 2);
@@ -539,7 +539,7 @@ if.then:
539539
auto IR = llvm::InlineFunction(*CB, IFI);
540540
EXPECT_TRUE(IR.isSuccess());
541541
invalidate(*F1);
542-
FPU.finish(FAM);
542+
EXPECT_TRUE(FPU.finishAndTest(FAM));
543543
EXPECT_EQ(static_cast<size_t>(FPI.BasicBlockCount), F1->size() - 1);
544544
EXPECT_EQ(static_cast<size_t>(FPI.TotalInstructionCount),
545545
F1->getInstructionCount() - 2);
@@ -610,7 +610,7 @@ define i32 @f2(i32 %a) {
610610
auto IR = llvm::InlineFunction(*CB, IFI);
611611
EXPECT_TRUE(IR.isSuccess());
612612
invalidate(*F1);
613-
FPU.finish(FAM);
613+
EXPECT_TRUE(FPU.finishAndTest(FAM));
614614
EXPECT_EQ(FPI, ExpectedFinal);
615615
}
616616

@@ -677,7 +677,7 @@ declare void @llvm.trap()
677677
auto IR = llvm::InlineFunction(*CB, IFI);
678678
EXPECT_TRUE(IR.isSuccess());
679679
invalidate(*F1);
680-
FPU.finish(FAM);
680+
EXPECT_TRUE(FPU.finishAndTest(FAM));
681681
EXPECT_EQ(FPI, ExpectedFinal);
682682
}
683683

@@ -737,7 +737,7 @@ declare void @f3()
737737
auto IR = llvm::InlineFunction(*CB, IFI);
738738
EXPECT_TRUE(IR.isSuccess());
739739
invalidate(*F1);
740-
FPU.finish(FAM);
740+
EXPECT_TRUE(FPU.finishAndTest(FAM));
741741
EXPECT_EQ(FPI, ExpectedFinal);
742742
}
743743
} // end anonymous namespace

0 commit comments

Comments
 (0)