Skip to content

Commit 47a4ad2

Browse files
atrickatrick
authored
Merge pull request swiftlang#35531 from atrick/ossa-enforcementopts
Enable AccessEnforcementOpts with OSSA
2 parents 8ec152e + 454a445 commit 47a4ad2

File tree

5 files changed

+1888
-6
lines changed

5 files changed

+1888
-6
lines changed

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,7 +1134,6 @@ bool SILInstruction::mayRelease() const {
11341134
return true;
11351135

11361136
case SILInstructionKind::DestroyValueInst:
1137-
assert(!SILModuleConventions(getModule()).useLoweredAddresses());
11381137
return true;
11391138

11401139
case SILInstructionKind::UnconditionalCheckedCastAddrInst:

‎lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,9 @@ void addHighLevelLoopOptPasses(SILPassPipelinePlan &P) {
242242
P.addSimplifyCFG();
243243
// Optimize access markers for better LICM: might merge accesses
244244
// It will also set the no_nested_conflict for dynamic accesses
245+
// AccessEnforcementReleaseSinking results in non-canonical OSSA.
246+
// It is only used to expose opportunities in AccessEnforcementOpts
247+
// before CanonicalOSSA re-hoists destroys.
245248
P.addAccessEnforcementReleaseSinking();
246249
P.addAccessEnforcementOpts();
247250
P.addHighLevelLICM();
@@ -250,6 +253,9 @@ void addHighLevelLoopOptPasses(SILPassPipelinePlan &P) {
250253
// LICM might have added new merging potential by hoisting
251254
// we don't want to restart the pipeline - ignore the
252255
// potential of merging out of two loops
256+
// AccessEnforcementReleaseSinking results in non-canonical OSSA.
257+
// It is only used to expose opportunities in AccessEnforcementOpts
258+
// before CanonicalOSSA re-hoists destroys.
253259
P.addAccessEnforcementReleaseSinking();
254260
P.addAccessEnforcementOpts();
255261
// Start of loop unrolling passes.

‎lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
/// - Should run immediately before the AccessEnforcementWMO to share
1818
/// AccessedStorageAnalysis results.
1919
///
20+
/// - Benefits from running after AccessEnforcementReleaseSinking.
21+
///
2022
/// This pass optimizes access enforcement as follows:
2123
///
2224
/// **Access marker folding**
@@ -727,7 +729,7 @@ void AccessConflictAndMergeAnalysis::visitFullApply(FullApplySite fullApply,
727729
ASA->getCallSiteEffects(callSiteAccesses, fullApply);
728730

729731
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *fullApply.getInstruction()
730-
<< " call site accesses: ";
732+
<< " call site accesses:\n";
731733
callSiteAccesses.dump());
732734
recordConflicts(state, callSiteAccesses.getResult());
733735
}
@@ -1094,10 +1096,6 @@ struct AccessEnforcementOpts : public SILFunctionTransform {
10941096
if (F->empty())
10951097
return;
10961098

1097-
// FIXME: Support ownership.
1098-
if (F->hasOwnership())
1099-
return;
1100-
11011099
LLVM_DEBUG(llvm::dbgs() << "Running local AccessEnforcementOpts on "
11021100
<< F->getName() << "\n");
11031101

‎test/SILOptimizer/access_enforcement_fastpath_ossa.sil

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// RUN: %target-sil-opt -access-enforcement-opts -debug-only=access-enforcement-opts -verify %s -o /dev/null 2>&1 | %FileCheck %s
2+
//
3+
// REQUIRES: asserts
4+
//
5+
// This tests four kinds of AccessEnforcementOpts fast paths:
6+
//
7+
// 1. During identifyAccess, determine if there are either any
8+
// identical accesses or an accesses that aren't already marked
9+
// no_nested_storage. If there are neither, then skip the subsequent
10+
// data flow analysis.
11+
//
12+
// 2. In StorageSet, indicate whether identical storage was seen
13+
// elsewhere in the function. During dataflow, only add an access to
14+
// the out-of-scope access set if was marked as having identical
15+
// storage with another access.
16+
//
17+
// 3. If no access pairs can be merged, avoid analyzing SCC regions in
18+
// the control flow graph.
19+
//
20+
// 4. During data flow, don't track in scope conflicts for
21+
// instructions already marked [no_nested_conflict].
22+
23+
sil_stage canonical
24+
25+
import Builtin
26+
import Swift
27+
import SwiftShims
28+
29+
public class TestEarlyBail {
30+
@_hasStorage var prop1: Int { get set }
31+
@_hasStorage var prop2: Int { get set }
32+
}
33+
34+
// All accesses are no_nested_conflict, and none are identical. Bail.
35+
// (1) Skipping AccessConflictAndMergeAnalysis...
36+
//
37+
// CHECK-LABEL: Running local AccessEnforcementOpts on testEarlyBail
38+
// CHECK-NOT: Processing
39+
// CHECK-NOT: No conflict on one path
40+
// CHECK-NOT: Folding
41+
// CHECK-NOT: Merging
42+
// CHECK: Skipping AccessConflictAndMergeAnalysis...
43+
sil [ossa] @testEarlyBail : $@convention(method) (@owned TestEarlyBail) -> @owned TestEarlyBail {
44+
bb0(%0 : @owned $TestEarlyBail):
45+
%borrow = begin_borrow %0 : $TestEarlyBail
46+
%adr1 = ref_element_addr %borrow : $TestEarlyBail, #TestEarlyBail.prop1
47+
%a1 = begin_access [modify] [dynamic] [no_nested_conflict] %adr1 : $*Int
48+
%v1 = load [trivial] %a1 : $*Int
49+
end_access %a1 : $*Int
50+
%adr2 = ref_element_addr %borrow : $TestEarlyBail, #TestEarlyBail.prop2
51+
%a2 = begin_access [modify] [dynamic] [no_nested_conflict] %adr2 : $*Int
52+
%v2 = load [trivial] %a2 : $*Int
53+
end_access %a2 : $*Int
54+
end_borrow %borrow : $TestEarlyBail
55+
return %0 : $TestEarlyBail
56+
}
57+
58+
// An access is not marked no_nested_conflict. Need to run analysis.
59+
// NOT (1) Skipping AccessConflictAndMergeAnalysis...
60+
// (2) Ignoring unique access...
61+
// (3) Skipping SCC analysis...
62+
//
63+
// CHECK-LABEL: Running local AccessEnforcementOpts on testEarlyBailNeedsFolding
64+
// CHECK: Processing Function: testEarlyBailNeedsFolding
65+
// CHECK: No conflict on one path from [[ACCESS:%.*]] = begin_access [modify] [dynamic] %{{.*}} : $*Int
66+
// CHECK-NEXT: to end_access [[ACCESS]] : $*Int
67+
// CHECK: Ignoring unmergeable access: [[ACCESS]] = begin_access [modify] [dynamic] %{{.*}} : $*Int
68+
// CHECK: Folding [[ACCESS]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
69+
// CHECK: Skipping SCC Analysis...
70+
sil [ossa] @testEarlyBailNeedsFolding : $@convention(method) (@owned TestEarlyBail) -> @owned TestEarlyBail {
71+
bb0(%0 : @owned $TestEarlyBail):
72+
%borrow = begin_borrow %0 : $TestEarlyBail
73+
%adr1 = ref_element_addr %borrow : $TestEarlyBail, #TestEarlyBail.prop1
74+
%a1 = begin_access [modify] [dynamic] %adr1 : $*Int
75+
%v1 = load [trivial] %a1 : $*Int
76+
end_access %a1 : $*Int
77+
end_borrow %borrow : $TestEarlyBail
78+
return %0 : $TestEarlyBail
79+
}
80+
81+
// Two accesses have the same storage. Need to run analysis.
82+
// NOT (1) Skipping AccessConflictAndMergeAnalysis...
83+
// NOT (2) Ignoring unique access...
84+
// (4) No Conflict On Path
85+
// (4) NOT No Conflict On Path
86+
// NOT (3) Skipping SCC analysis...
87+
//
88+
// CHECK-LABEL: Running local AccessEnforcementOpts on testEarlyBailMayMerge
89+
// CHECK: Processing Function: testEarlyBailMayMerge
90+
// CHECK: No conflict on one path from [[ACCESS1:%.*]] = begin_access [modify] [dynamic] %{{.*}} : $*Int
91+
// CHECK: to end_access [[ACCESS1]] : $*Int
92+
// CHECK: Found mergable pair: [[ACCESS1]] = begin_access [modify] [dynamic] %{{.*}} : $*Int
93+
// CHECK-NEXT: with [[ACCESS2:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
94+
// CHECK-NOT: No conflict on one path from [[ACCESS2]]
95+
// CHECK: Ignoring unmergeable access: [[ACCESS2]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
96+
// CHECK-NOT: No conflict on one path from [[ACCESS2]]
97+
//
98+
// Note: The order that accesses with no nested conflicts are "Folded" is nondeterministic.
99+
// CHECK-DAG: Folding [[ACCESS1]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
100+
// CHECK-DAG: Folding [[ACCESS2]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
101+
// CHECK: Merging [[ACCESS2]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
102+
// CHECK-NEXT: into [[ACCESS1]] = begin_access [modify] [dynamic] [no_nested_conflict] %{{.*}} : $*Int
103+
// CHECK-NOT: Skipping SCC Analysis...
104+
sil [ossa] @testEarlyBailMayMerge : $@convention(method) (@owned TestEarlyBail) -> @owned TestEarlyBail {
105+
bb0(%0 : @owned $TestEarlyBail):
106+
%borrow = begin_borrow %0 : $TestEarlyBail
107+
%adr1 = ref_element_addr %borrow : $TestEarlyBail, #TestEarlyBail.prop1
108+
%a1 = begin_access [modify] [dynamic] %adr1 : $*Int
109+
%v1 = load [trivial] %a1 : $*Int
110+
end_access %a1 : $*Int
111+
%adr2 = ref_element_addr %borrow : $TestEarlyBail, #TestEarlyBail.prop1
112+
%a2 = begin_access [modify] [dynamic] [no_nested_conflict] %adr2 : $*Int
113+
%v2 = load [trivial] %a2 : $*Int
114+
end_access %a2 : $*Int
115+
end_borrow %borrow : $TestEarlyBail
116+
return %0 : $TestEarlyBail
117+
}

0 commit comments

Comments
 (0)