Skip to content

Commit 2be41e7

Browse files
nikicnikic
authored
[AlwaysInline] Fix analysis invalidation (#119566)
This is a followup to #117750. Currently, AlwaysInline only invalidates analyses at the end, by returning that no analyses are preserved. However, this means that analyses fetched during inlining may be outdated. The aforementioned PR exposed this issue. Instead, bring the logic closer to what the normal inliner does, by directly invalidating the caller in FAM. This should make sure that we don't receive any outdated analyses even if they are fetched during inlining. Also drop the BFI updating entirely -- there's no point in doing it if we're going to invalidate everything anyway.
1 parent 9472c5f commit 2be41e7

File tree

2 files changed

+120
-23
lines changed

2 files changed

+120
-23
lines changed

‎llvm/lib/Transforms/IPO/AlwaysInliner.cpp‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,9 @@ namespace {
3232

3333
bool AlwaysInlineImpl(
3434
Module &M, bool InsertLifetime, ProfileSummaryInfo &PSI,
35+
FunctionAnalysisManager *FAM,
3536
function_ref<AssumptionCache &(Function &)> GetAssumptionCache,
36-
function_ref<AAResults &(Function &)> GetAAR,
37-
function_ref<BlockFrequencyInfo &(Function &)> GetBFI,
38-
function_ref<BlockFrequencyInfo *(Function &)> GetCachedBFI) {
37+
function_ref<AAResults &(Function &)> GetAAR) {
3938
SmallSetVector<CallBase *, 16> Calls;
4039
bool Changed = false;
4140
SmallVector<Function *, 16> InlinedComdatFunctions;
@@ -62,12 +61,7 @@ bool AlwaysInlineImpl(
6261
DebugLoc DLoc = CB->getDebugLoc();
6362
BasicBlock *Block = CB->getParent();
6463

65-
// Only update CallerBFI if already available. The CallerBFI update
66-
// requires CalleeBFI.
67-
BlockFrequencyInfo *CallerBFI = GetCachedBFI(*Caller);
68-
InlineFunctionInfo IFI(GetAssumptionCache, &PSI, CallerBFI,
69-
CallerBFI ? &GetBFI(F) : nullptr);
70-
64+
InlineFunctionInfo IFI(GetAssumptionCache, &PSI, nullptr, nullptr);
7165
InlineResult Res = InlineFunction(*CB, IFI, /*MergeAttributes=*/true,
7266
&GetAAR(F), InsertLifetime);
7367
if (!Res.isSuccess()) {
@@ -86,6 +80,8 @@ bool AlwaysInlineImpl(
8680
/*ForProfileContext=*/false, DEBUG_TYPE);
8781

8882
Changed = true;
83+
if (FAM)
84+
FAM->invalidate(*Caller, PreservedAnalyses::none());
8985
}
9086

9187
F.removeDeadConstantUsers();
@@ -95,6 +91,8 @@ bool AlwaysInlineImpl(
9591
if (F.hasComdat()) {
9692
InlinedComdatFunctions.push_back(&F);
9793
} else {
94+
if (FAM)
95+
FAM->clear(F, F.getName());
9896
M.getFunctionList().erase(F);
9997
Changed = true;
10098
}
@@ -107,6 +105,8 @@ bool AlwaysInlineImpl(
107105
filterDeadComdatFunctions(InlinedComdatFunctions);
108106
// The remaining functions are actually dead.
109107
for (Function *F : InlinedComdatFunctions) {
108+
if (FAM)
109+
FAM->clear(*F, F->getName());
110110
M.getFunctionList().erase(F);
111111
Changed = true;
112112
}
@@ -136,12 +136,9 @@ struct AlwaysInlinerLegacyPass : public ModulePass {
136136
auto GetAssumptionCache = [&](Function &F) -> AssumptionCache & {
137137
return getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
138138
};
139-
auto GetCachedBFI = [](Function &) -> BlockFrequencyInfo * {
140-
return nullptr;
141-
};
142139

143-
return AlwaysInlineImpl(M, InsertLifetime, PSI, GetAssumptionCache, GetAAR,
144-
/*GetBFI=*/nullptr, GetCachedBFI);
140+
return AlwaysInlineImpl(M, InsertLifetime, PSI, /*FAM=*/nullptr,
141+
GetAssumptionCache, GetAAR);
145142
}
146143

147144
static char ID; // Pass identification, replacement for typeid
@@ -175,19 +172,18 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
175172
auto GetAssumptionCache = [&](Function &F) -> AssumptionCache & {
176173
return FAM.getResult<AssumptionAnalysis>(F);
177174
};
178-
auto GetBFI = [&](Function &F) -> BlockFrequencyInfo & {
179-
return FAM.getResult<BlockFrequencyAnalysis>(F);
180-
};
181-
auto GetCachedBFI = [&](Function &F) -> BlockFrequencyInfo * {
182-
return FAM.getCachedResult<BlockFrequencyAnalysis>(F);
183-
};
184175
auto GetAAR = [&](Function &F) -> AAResults & {
185176
return FAM.getResult<AAManager>(F);
186177
};
187178
auto &PSI = MAM.getResult<ProfileSummaryAnalysis>(M);
188179

189-
bool Changed = AlwaysInlineImpl(M, InsertLifetime, PSI, GetAssumptionCache,
190-
GetAAR, GetBFI, GetCachedBFI);
180+
bool Changed = AlwaysInlineImpl(M, InsertLifetime, PSI, &FAM,
181+
GetAssumptionCache, GetAAR);
182+
if (!Changed)
183+
return PreservedAnalyses::all();
191184

192-
return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
185+
PreservedAnalyses PA;
186+
// We have already invalidated all analyses on modified functions.
187+
PA.preserveSet<AllAnalysesOn<Function>>();
188+
return PA;
193189
}

‎llvm/test/Transforms/Inline/always-inline-bfi.ll‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -passes="function(require<block-freq>,loop(loop-unroll-full)),always-inline" < %s | FileCheck %s
3+
4+
; Make sure this does not crash.
5+
6+
define void @f_116_0(ptr %p) alwaysinline {
7+
; CHECK-LABEL: define void @f_116_0(
8+
; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0:[0-9]+]] {
9+
; CHECK-NEXT: [[ENTRY:.*:]]
10+
; CHECK-NEXT: [[DOTPRE:%.*]] = load i16, ptr [[P]], align 1
11+
; CHECK-NEXT: br label %[[FOR_COND:.*]]
12+
; CHECK: [[FOR_COND]]:
13+
; CHECK-NEXT: [[CMP3:%.*]] = icmp ult i16 [[DOTPRE]], 1
14+
; CHECK-NEXT: br i1 [[CMP3]], label %[[FOR_BODY:.*]], label %[[FOR_COND_CLEANUP:.*]]
15+
; CHECK: [[FOR_COND_CLEANUP]]:
16+
; CHECK-NEXT: ret void
17+
; CHECK: [[FOR_BODY]]:
18+
; CHECK-NEXT: br label %[[FOR_COND]]
19+
;
20+
entry:
21+
%.pre = load i16, ptr %p, align 1
22+
br label %for.cond
23+
24+
for.cond: ; preds = %for.body, %entry
25+
%cmp3 = icmp ult i16 %.pre, 1
26+
br i1 %cmp3, label %for.body, label %for.cond.cleanup
27+
28+
for.cond.cleanup: ; preds = %for.cond
29+
ret void
30+
31+
for.body: ; preds = %for.cond
32+
br label %for.cond
33+
}
34+
35+
define void @f_321_0(ptr %p) alwaysinline {
36+
; CHECK-LABEL: define void @f_321_0(
37+
; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0]] {
38+
; CHECK-NEXT: [[ENTRY:.*:]]
39+
; CHECK-NEXT: br label %[[FOR_COND:.*]]
40+
; CHECK: [[FOR_COND]]:
41+
; CHECK-NEXT: br i1 false, label %[[CRIT_EDGE:.*]], label %[[FOR_COND_CLEANUP:.*]]
42+
; CHECK: [[CRIT_EDGE]]:
43+
; CHECK-NEXT: unreachable
44+
; CHECK: [[FOR_COND_CLEANUP]]:
45+
; CHECK-NEXT: [[DOTPRE_I:%.*]] = load i16, ptr [[P]], align 1
46+
; CHECK-NEXT: br label %[[FOR_COND_I:.*]]
47+
; CHECK: [[FOR_COND_I]]:
48+
; CHECK-NEXT: [[CMP3_I:%.*]] = icmp ult i16 [[DOTPRE_I]], 1
49+
; CHECK-NEXT: br i1 [[CMP3_I]], label %[[FOR_BODY_I:.*]], label %[[F_116_0_EXIT:.*]]
50+
; CHECK: [[FOR_BODY_I]]:
51+
; CHECK-NEXT: br label %[[FOR_COND_I]]
52+
; CHECK: [[F_116_0_EXIT]]:
53+
; CHECK-NEXT: ret void
54+
;
55+
entry:
56+
br label %for.cond
57+
58+
for.cond: ; preds = %crit_edge, %entry
59+
br i1 false, label %crit_edge, label %for.cond.cleanup
60+
61+
crit_edge: ; preds = %for.cond
62+
br label %for.cond
63+
64+
for.cond.cleanup: ; preds = %for.cond
65+
call void @f_116_0(ptr %p)
66+
ret void
67+
}
68+
69+
define i16 @main(ptr %p) {
70+
; CHECK-LABEL: define i16 @main(
71+
; CHECK-SAME: ptr [[P:%.*]]) {
72+
; CHECK-NEXT: [[ENTRY:.*:]]
73+
; CHECK-NEXT: br label %[[FOR_COND:.*]]
74+
; CHECK: [[FOR_COND]]:
75+
; CHECK-NEXT: br label %[[FOR_COND]]
76+
; CHECK: [[IF_ELSE:.*:]]
77+
; CHECK-NEXT: [[DOTPRE_I_I:%.*]] = load i16, ptr [[P]], align 1
78+
; CHECK-NEXT: br label %[[FOR_COND_I_I:.*]]
79+
; CHECK: [[FOR_COND_I_I]]:
80+
; CHECK-NEXT: [[CMP3_I_I:%.*]] = icmp ult i16 [[DOTPRE_I_I]], 1
81+
; CHECK-NEXT: br i1 [[CMP3_I_I]], label %[[FOR_BODY_I_I:.*]], label %[[F_321_0_EXIT:.*]]
82+
; CHECK: [[FOR_BODY_I_I]]:
83+
; CHECK-NEXT: br label %[[FOR_COND_I_I]]
84+
; CHECK: [[F_321_0_EXIT]]:
85+
; CHECK-NEXT: br label %[[FOR_COND115:.*]]
86+
; CHECK: [[FOR_COND115]]:
87+
; CHECK-NEXT: br label %[[FOR_COND115]]
88+
;
89+
entry:
90+
br label %for.cond
91+
92+
for.cond: ; preds = %for.cond, %entry
93+
br label %for.cond
94+
95+
if.else: ; No predecessors!
96+
call void @f_321_0(ptr %p)
97+
br label %for.cond115
98+
99+
for.cond115: ; preds = %for.cond115, %if.else
100+
br label %for.cond115
101+
}

0 commit comments

Comments
 (0)