Skip to content

Commit d84de12

Browse files
committed
Revert "Change FSO explosion heuristic"
This reverts commit fa09c6b. Broke Linux build. And also PR "please benchmark" does not seem to catch it.
1 parent 76ce49c commit d84de12

File tree

6 files changed

+3
-102
lines changed

6 files changed

+3
-102
lines changed

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,6 @@ 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-
193189
bool HasBlock = false;
194190

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

226222
bool hasBlock() const { return HasBlock; }
227223

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-
234224
bool isSingleRelease(SILArgument *Arg) const {
235225
auto Iter = ArgInstMap.find(Arg);
236226
assert(Iter != ArgInstMap.end() && "Failed to get release list for argument");

include/swift/SILOptimizer/Utils/FunctionSignatureOptUtils.h

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

125125
/// Return true if it's both legal and a good idea to explode this argument.
126-
bool shouldExplode(ConsumedArgToEpilogueReleaseMatcher &ERM) const {
126+
bool shouldExplode() 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-
142131
// See if the projection tree consists of potentially multiple levels of
143132
// structs containing one field. In such a case, there is no point in
144133
// exploding the argument.

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -739,10 +739,6 @@ 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-
746742
ArgToRemove.insert(Arg.first);
747743
}
748744

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,6 @@
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-
//===----------------------------------------------------------------------===//
2212

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

552-
553542
SILFunction *F = getFunction();
554543
llvm::BumpPtrAllocator Allocator;
555544
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(ArgToReturnReleaseMap);
263+
A.Explode = A.shouldExplode();
264264

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

test/SILOptimizer/functionsigopts.sil

Lines changed: 1 addition & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -42,64 +42,8 @@ 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-
5245
sil @use_Int : $@convention(thin) (Int) -> ()
5346

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-
10347
// We do this check separately to ensure that this does not occur anywhere in
10448
// the output for the file. (Maybe this is an interesting enough feature to add to FileCheck?).
10549

@@ -230,7 +174,6 @@ bb0(%0 : $boo):
230174
return %4 : $(Int, Int)
231175
}
232176

233-
234177
// CHECK-LABEL: sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
235178
// CHECK-NOT: function_ref @_TTSf4d_n__dead_arg_no_callsites
236179
sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
@@ -777,13 +720,7 @@ bb0(%0 : $boo):
777720
return %5 : $()
778721
}
779722

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-
}
723+
787724

788725

789726
//========================================

0 commit comments

Comments
 (0)