Skip to content

Commit fa09c6b

Browse files
committed
Change FSO explosion heuristic
If we can not find the epilogue releases for all the fields with reference sematics, but we found for some fields. Explode the argument. I do not see a performance improvement with this change rdar://25451364
1 parent 7dc6855 commit fa09c6b

File tree

6 files changed

+102
-3
lines changed

6 files changed

+102
-3
lines changed

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,10 @@ class ConsumedArgToEpilogueReleaseMatcher {
186186
RCIdentityFunctionInfo *RCFI;
187187
ExitKind Kind;
188188
llvm::SmallMapVector<SILArgument *, ReleaseList, 8> ArgInstMap;
189+
190+
/// Set to true if we found some releases but not all for the argument.
191+
llvm::DenseSet<SILArgument *> FoundSomeReleases;
192+
189193
bool HasBlock = false;
190194

191195
/// Return true if we have seen releases to part or all of \p Derived in
@@ -221,6 +225,12 @@ class ConsumedArgToEpilogueReleaseMatcher {
221225

222226
bool hasBlock() const { return HasBlock; }
223227

228+
/// Return true if we've found some epilgoue releases for the argument
229+
/// but not all.
230+
bool hasSomeReleasesForArgument(SILArgument *Arg) {
231+
return FoundSomeReleases.find(Arg) != FoundSomeReleases.end();
232+
}
233+
224234
bool isSingleRelease(SILArgument *Arg) const {
225235
auto Iter = ArgInstMap.find(Arg);
226236
assert(Iter != ArgInstMap.end() && "Failed to get release list for argument");

include/swift/SILOptimizer/Utils/FunctionSignatureOptUtils.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,22 @@ struct ArgumentDescriptor {
123123
}
124124

125125
/// Return true if it's both legal and a good idea to explode this argument.
126-
bool shouldExplode() const {
126+
bool shouldExplode(ConsumedArgToEpilogueReleaseMatcher &ERM) const {
127127
// We cannot optimize the argument.
128128
if (!canOptimizeLiveArg())
129129
return false;
130130

131+
// If this argument is @owned and we can not find all the releases for it
132+
// try to explode it, maybe we can find some of the releases and O2G some
133+
// of its components.
134+
//
135+
// This is a potentially a very profitable optimization. Ignore other
136+
// heuristics.
137+
if (hasConvention(SILArgumentConvention::Direct_Owned)) {
138+
if (ERM.hasSomeReleasesForArgument(Arg))
139+
return true;
140+
}
141+
131142
// See if the projection tree consists of potentially multiple levels of
132143
// structs containing one field. In such a case, there is no point in
133144
// exploding the argument.

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,10 @@ processMatchingReleases() {
739739
if (releaseArgument(Arg.second, Arg.first))
740740
continue;
741741

742+
// OK. we did find some epilogue releases, just not all.
743+
if (!Arg.second.empty())
744+
FoundSomeReleases.insert(Arg.first);
745+
742746
ArgToRemove.insert(Arg.first);
743747
}
744748

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@
99
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
1010
//
1111
//===----------------------------------------------------------------------===//
12+
//
13+
// I looked through many functions in the Stdlib. This is what i feel how FSO
14+
// can be further improved.
15+
//
16+
// TODO: Most of the O2G for function arguments are done. but we are still
17+
// missing a lot of things on O2G for return result. We maybe able to improve
18+
// the ConsumedResultToEpilogueRetainMatcher. Right now it is kept conservative
19+
// in present of complex control flows and release instructions.
20+
//
21+
//===----------------------------------------------------------------------===//
1222

1323
#define DEBUG_TYPE "sil-function-signature-opt"
1424
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
@@ -539,6 +549,7 @@ class FunctionSignatureOpts : public SILFunctionTransform {
539549
auto *AA = PM->getAnalysis<AliasAnalysis>();
540550
auto *CA = PM->getAnalysis<CallerAnalysis>();
541551

552+
542553
SILFunction *F = getFunction();
543554
llvm::BumpPtrAllocator Allocator;
544555
FunctionSignatureInfo FSI(F, Allocator, AA, RCIA->get(F));

lib/SILOptimizer/Utils/FunctionSignatureOptUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ bool FunctionSignatureInfo::analyzeParameters() {
260260
bool HaveOptimizedArg = false;
261261

262262
// Whether we will explode the argument or not.
263-
A.Explode = A.shouldExplode();
263+
A.Explode = A.shouldExplode(ArgToReturnReleaseMap);
264264

265265
bool isABIRequired = isArgumentABIRequired(Args[i]);
266266
auto OnlyRelease = getNonTrivialNonDebugReleaseUse(Args[i]);

test/SILOptimizer/functionsigopts.sil

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,64 @@ struct lotsoffield {
4242
init()
4343
}
4444

45+
struct goo {
46+
var left : foo
47+
var right : foo
48+
var top : foo
49+
var bottom : foo
50+
}
51+
4552
sil @use_Int : $@convention(thin) (Int) -> ()
4653

54+
55+
// CHECK-LABEL: sil [thunk] [always_inline] @argument_with_incomplete_epilogue_release
56+
// CHECK: [[IN2:%.*]] = struct_extract [[IN1:%.*]] : $goo, #goo.top
57+
// CHECK: function_ref @_TTSf4g_n___TTSf4s__argument_with_incomplete_epilogue_release : $@convention(thin) (@guaranteed foo, @owned foo) -> ()
58+
// CHECK: release_value [[IN2]]
59+
sil @argument_with_incomplete_epilogue_release : $@convention(thin) (@owned goo) -> () {
60+
bb0(%0 : $goo):
61+
// make inline costs = 2
62+
%c1 = builtin "assert_configuration"() : $Builtin.Int32
63+
%c2 = builtin "assert_configuration"() : $Builtin.Int32
64+
%c3 = builtin "assert_configuration"() : $Builtin.Int32
65+
%c4 = builtin "assert_configuration"() : $Builtin.Int32
66+
%c5 = builtin "assert_configuration"() : $Builtin.Int32
67+
%c6 = builtin "assert_configuration"() : $Builtin.Int32
68+
%c7 = builtin "assert_configuration"() : $Builtin.Int32
69+
%c8 = builtin "assert_configuration"() : $Builtin.Int32
70+
%c9 = builtin "assert_configuration"() : $Builtin.Int32
71+
%c10 = builtin "assert_configuration"() : $Builtin.Int32
72+
%c11 = builtin "assert_configuration"() : $Builtin.Int32
73+
%c12 = builtin "assert_configuration"() : $Builtin.Int32
74+
%c13 = builtin "assert_configuration"() : $Builtin.Int32
75+
%c14 = builtin "assert_configuration"() : $Builtin.Int32
76+
%c15 = builtin "assert_configuration"() : $Builtin.Int32
77+
%c16 = builtin "assert_configuration"() : $Builtin.Int32
78+
%c17 = builtin "assert_configuration"() : $Builtin.Int32
79+
%c18 = builtin "assert_configuration"() : $Builtin.Int32
80+
%c19 = builtin "assert_configuration"() : $Builtin.Int32
81+
%c20 = builtin "assert_configuration"() : $Builtin.Int32
82+
%c21 = builtin "assert_configuration"() : $Builtin.Int32
83+
%c22 = builtin "assert_configuration"() : $Builtin.Int32
84+
85+
86+
87+
%1 = struct_extract %0 : $goo, #goo.top
88+
%2 = ref_element_addr %1 : $foo, #foo.a
89+
%3 = load %2 : $*Int
90+
%4 = function_ref @use_Int : $@convention(thin) (Int) -> ()
91+
apply %4(%3) : $@convention(thin) (Int) -> ()
92+
93+
%5 = struct_extract %0 : $goo, #goo.bottom
94+
%6 = ref_element_addr %5 : $foo, #foo.a
95+
%7 = load %6 : $*Int
96+
apply %4(%7) : $@convention(thin) (Int) -> ()
97+
98+
release_value %1 : $foo
99+
%8 = tuple ()
100+
return %8 : $()
101+
}
102+
47103
// We do this check separately to ensure that this does not occur anywhere in
48104
// the output for the file. (Maybe this is an interesting enough feature to add to FileCheck?).
49105

@@ -174,6 +230,7 @@ bb0(%0 : $boo):
174230
return %4 : $(Int, Int)
175231
}
176232

233+
177234
// CHECK-LABEL: sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
178235
// CHECK-NOT: function_ref @_TTSf4d_n__dead_arg_no_callsites
179236
sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
@@ -720,7 +777,13 @@ bb0(%0 : $boo):
720777
return %5 : $()
721778
}
722779

723-
780+
sil [fragile] @argument_with_incomplete_epilogue_release_callsite : $@convention(thin) (@owned goo) -> () {
781+
bb0(%0 : $goo):
782+
%2 = function_ref @argument_with_incomplete_epilogue_release : $@convention(thin) (@owned goo) -> ()
783+
%4 = apply %2(%0) : $@convention(thin) (@owned goo) -> ()
784+
%5 = tuple()
785+
return %5 : $()
786+
}
724787

725788

726789
//========================================

0 commit comments

Comments
 (0)