llvm.org GIT mirror llvm / f4c6127
[PM] Fix new LoopUnroll function pass by invalidating loop analysis results when a loop is completely removed. This is very hard to manifest as a visible bug. You need to arrange for there to be a subsequent allocation of a 'Loop' object which gets the exact same address as the one which the unroll deleted, and you need the LoopAccessAnalysis results to be significant in the way that they're stale. And you need a million other things to align. But when it does, you get a deeply mysterious crash due to actually finding a stale analysis result. This fixes the issue and tests for it by directly checking we successfully invalidate things. I have not been able to get *any* test case to reliably trigger this. Changes to LLVM itself caused the only test case I ever had to cease to crash. I've looked pretty extensively at less brittle ways of fixing this and they are actually very, very hard to do. This is a somewhat strange and unusual case as we have a pass which is deleting an IR unit, but is not running within that IR unit's pass framework (which is what handles this cleanly for the normal loop unroll). And where there isn't a definitive way to clear *all* of the stale cache entries. And where the pass *is* updating the core analysis that provides the IR units! For example, we don't have any of these problems with Function analyses because it is easy to clear out function analyses when the functions themselves may have been deleted -- we clear an entire module's worth! But that is too heavy of a hammer down here in the LoopAnalysisManager layer. A better long-term solution IMO is to require that AnalysisManager's make their keys durable to this kind of thing. Specifically, when caching an analysis for one IR unit that is conceptually "owned" by a higher level IR unit, the AnalysisManager should incorporate this into its data structures so that we can reliably clear these results without having to teach each and every pass to do so manually as we do here. But that is a change for another day as it will be a fairly invasive change to the AnalysisManager infrastructure. Until then, this fortunately seems to be quite rare. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310333 91177308-0d34-0410-b5e6-96231b3b80d8 Chandler Carruth 2 years ago
2 changed file(s) with 125 addition(s) and 2 deletion(s). Raw diff Collapse all Expand all
12521252 auto &AC = AM.getResult(F);
12531253 auto &ORE = AM.getResult(F);
12541254
1255 LoopAnalysisManager *LAM = nullptr;
1256 if (auto *LAMProxy = AM.getCachedResult(F))
1257 LAM = &LAMProxy->getManager();
1258
12551259 const ModuleAnalysisManager &MAM =
12561260 AM.getResult(F).getManager();
12571261 ProfileSummaryInfo *PSI =
12771281 // for unrolling is only needed to get optimization remarks emitted in
12781282 // a forward order.
12791283 Loop &L = *Worklist.pop_back_val();
1280 #ifndef NDEBUG
12811284 Loop *ParentL = L.getParentLoop();
1282 #endif
12831285
12841286 // The API here is quite complex to call, but there are only two interesting
12851287 // states we support: partial and full (or "simple") unrolling. However, to
13041306 if (CurChanged && ParentL)
13051307 ParentL->verifyLoop();
13061308 #endif
1309
1310 // Walk the parent or top-level loops after unrolling to check whether we
1311 // actually removed a loop, and if so clear any cached analysis results for
1312 // it. We have to do this immediately as the next unrolling could allocate
1313 // a new loop object that ends up with the same address as the deleted loop
1314 // object causing cache collisions.
1315 if (LAM) {
1316 bool IsCurLoopValid =
1317 ParentL
1318 ? llvm::any_of(*ParentL, [&](Loop *SibL) { return SibL == &L; })
1319 : llvm::any_of(LI, [&](Loop *SibL) { return SibL == &L; });
1320 if (!IsCurLoopValid)
1321 LAM->clear(L);
1322 }
13071323 }
13081324
13091325 if (!Changed)
0 ; This test exercises that we don't corrupt a loop-analysis when running loop
1 ; unrolling in a way that deletes a loop. To do that, we first ensure the
2 ; analysis is cached, then unroll the loop (deleting it) and make sure that the
3 ; next function doesn't get a cache "hit" for this stale analysis result.
4 ;
5 ; RUN: opt -S -passes='loop(require),unroll,loop(print-access-info)' -debug-pass-manager < %s 2>&1 | FileCheck %s
6 ;
7 ; CHECK: Starting llvm::Function pass manager run.
8 ; CHECK: Running pass: FunctionToLoopPassAdaptor
9 ; CHECK: Running analysis: LoopAnalysis
10 ; CHECK: Running analysis: InnerAnalysisManagerProxy<{{.*}}LoopAnalysisManager
11 ; CHECK: Starting Loop pass manager run.
12 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
13 ; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
14 ; CHECK: Finished Loop pass manager run.
15 ; CHECK: Starting Loop pass manager run.
16 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
17 ; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
18 ; CHECK: Finished Loop pass manager run.
19 ; CHECK: Starting Loop pass manager run.
20 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
21 ; CHECK: Running analysis: LoopAccessAnalysis on outer.header
22 ; CHECK: Finished Loop pass manager run.
23 ; CHECK: Running pass: LoopUnrollPass
24 ; CHECK: Clearing all analysis results for: inner2.header
25 ; CHECK: Clearing all analysis results for: outer.header
26 ; CHECK: Invalidating all non-preserved analyses for: test
27 ; CHECK: Invalidating all non-preserved analyses for: inner1.header
28 ; CHECK: Invalidating analysis: LoopAccessAnalysis on inner1.header
29 ; CHECK: Invalidating all non-preserved analyses for: inner1.header.1
30 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on inner1.header.1
31 ; CHECK: Running pass: FunctionToLoopPassAdaptor
32 ; CHECK: Starting Loop pass manager run.
33 ; CHECK: Running pass: LoopAccessInfoPrinterPass
34 ; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
35 ; CHECK: Loop access info in function 'test':
36 ; CHECK: inner1.header:
37 ; CHECK: Finished Loop pass manager run.
38 ; CHECK: Starting Loop pass manager run.
39 ; CHECK: Running pass: LoopAccessInfoPrinterPass
40 ; CHECK: Running analysis: LoopAccessAnalysis on inner1.header.1
41 ; CHECK: Loop access info in function 'test':
42 ; CHECK: inner1.header.1:
43 ; CHECK: Finished Loop pass manager run.
44
45 target triple = "x86_64-unknown-linux-gnu"
46
47 define void @test(i32 %inner1.count) {
48 ; CHECK-LABEL: define void @test(
49 bb:
50 br label %outer.ph
51
52 outer.ph:
53 br label %outer.header
54
55 outer.header:
56 %outer.i = phi i32 [ 0, %outer.ph ], [ %outer.i.next, %outer.latch ]
57 br label %inner1.ph
58
59 inner1.ph:
60 br label %inner1.header
61
62 inner1.header:
63 %inner1.i = phi i32 [ 0, %inner1.ph ], [ %inner1.i.next, %inner1.header ]
64 %inner1.i.next = add i32 %inner1.i, 1
65 %inner1.cond = icmp eq i32 %inner1.i, %inner1.count
66 br i1 %inner1.cond, label %inner1.exit, label %inner1.header
67 ; We should have two unrolled copies of this loop and nothing else.
68 ;
69 ; CHECK-NOT: icmp eq
70 ; CHECK-NOT: br i1
71 ; CHECK: %[[COND1:.*]] = icmp eq i32 %{{.*}}, %inner1.count
72 ; CHECK: br i1 %[[COND1]],
73 ; CHECK-NOT: icmp eq
74 ; CHECK-NOT: br i1
75 ; CHECK: %[[COND2:.*]] = icmp eq i32 %{{.*}}, %inner1.count
76 ; CHECK: br i1 %[[COND2]],
77 ; CHECK-NOT: icmp eq
78 ; CHECK-NOT: br i1
79
80
81 inner1.exit:
82 br label %inner2.ph
83
84 inner2.ph:
85 br label %inner2.header
86
87 inner2.header:
88 %inner2.i = phi i32 [ 0, %inner2.ph ], [ %inner2.i.next, %inner2.header ]
89 %inner2.i.next = add i32 %inner2.i, 1
90 %inner2.cond = icmp eq i32 %inner2.i, 4
91 br i1 %inner2.cond, label %inner2.exit, label %inner2.header
92
93 inner2.exit:
94 br label %outer.latch
95
96 outer.latch:
97 %outer.i.next = add i32 %outer.i, 1
98 %outer.cond = icmp eq i32 %outer.i.next, 2
99 br i1 %outer.cond, label %outer.exit, label %outer.header
100
101 outer.exit:
102 br label %exit
103
104 exit:
105 ret void
106 }