Skip to content

Commit 279d058

Browse files
committed
[sil-combine] Canonicalize owned forwarding insts without non-debug non-consuming uses by sinking to their uses.
There are a bunch of optimizations in SILCombine where we try to fold an ownership forwarding instruction A into another ownership forwarding instruction B without deleting A. Consider the upcasts in the example below: ``` %0 = upcast %x : $X->Y %1 = upcast %0 : $Y->Z ``` These sorts of optimizations fold the first instruction into the second like so: ``` %0 = upcast %x : $X->Y %1 = upcast %x : $X->Z ``` This creates a problem when we are dealing with owned values since we have just introduced two consumes for %x. To work around this, we have two options: 1. Introduce extra copies. 2. We recognize the situations where we can guarantee that we can delete the first upcast. The first choice I believe is not a choice since breaking a forwarding chain of ownership in favor of extra copies is a less canonical form. That leaves us with the second form. What are the necessary/sufficient conditions for deleting the first upcast. Simply it is that the upcast cannot have any non-debug, non-consuming uses! In such a case, we know that along all paths through the program the value has exactly one non-debug use, one of its consuming uses. If when optimizing upcasts we could recognize that pattern, duplicate the inst along paths not through our 2nd upcast and thus delete the original upcast fixing the ownership error! While this is all nice and good there is a problem with this: it doesn't scale. As I was writing a few optimizations like this I began to note that I had to write different versions of this same helper for many of the visitors (they generally varied by how many forwarding instructions they looked through). As I pondered the above, I chatted a bit with @atrick and during our conversation, we both realized that it is much easier to solve this problem in one block and that the condition above would allow us to sink these instructiosn into the same block and thus if we could check for this condition and canonicialize the IR to sink these instructions before we visiting, we could use a single helper to handle all of these cases.
1 parent 56e9ca4 commit 279d058

File tree

4 files changed

+242
-1
lines changed

4 files changed

+242
-1
lines changed
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//===--- DebugOptUtils.h --------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
///
13+
/// Debug Info related utilities that rely on code in SILOptimizer/ and thus can
14+
/// not be in include/swift/SIL/DebugUtils.h.
15+
///
16+
//===----------------------------------------------------------------------===//
17+
18+
#ifndef SWIFT_SILOPTIMIZER_DEBUGOPTUTILS_H
19+
#define SWIFT_SILOPTIMIZER_DEBUGOPTUTILS_H
20+
21+
#include "swift/SIL/DebugUtils.h"
22+
#include "swift/SIL/SILValue.h"
23+
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
24+
25+
namespace swift {
26+
27+
/// Deletes all of the debug instructions that use \p value.
28+
inline void deleteAllDebugUses(SILValue value, InstModCallbacks &callbacks) {
29+
for (auto ui = value->use_begin(), ue = value->use_end(); ui != ue;) {
30+
auto *inst = ui->getUser();
31+
++ui;
32+
if (inst->isDebugInstruction()) {
33+
callbacks.deleteInst(inst);
34+
}
35+
}
36+
}
37+
38+
/// Deletes all of the debug uses of any result of \p inst.
39+
inline void deleteAllDebugUses(SILInstruction *inst,
40+
InstModCallbacks &callbacks) {
41+
for (SILValue v : inst->getResults()) {
42+
deleteAllDebugUses(v, callbacks);
43+
}
44+
}
45+
46+
} // namespace swift
47+
48+
#endif

lib/SILOptimizer/SILCombiner/SILCombine.cpp

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
//===----------------------------------------------------------------------===//
2020

2121
#define DEBUG_TYPE "sil-combine"
22+
2223
#include "SILCombiner.h"
2324
#include "swift/SIL/DebugUtils.h"
2425
#include "swift/SIL/SILBuilder.h"
@@ -28,6 +29,7 @@
2829
#include "swift/SILOptimizer/PassManager/Passes.h"
2930
#include "swift/SILOptimizer/PassManager/Transforms.h"
3031
#include "swift/SILOptimizer/Utils/CanonicalizeInstruction.h"
32+
#include "swift/SILOptimizer/Utils/DebugOptUtils.h"
3133
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
3234
#include "swift/SILOptimizer/Utils/SILOptFunctionBuilder.h"
3335
#include "swift/SILOptimizer/Utils/StackNesting.h"
@@ -41,6 +43,11 @@ using namespace swift;
4143
STATISTIC(NumCombined, "Number of instructions combined");
4244
STATISTIC(NumDeadInst, "Number of dead insts eliminated");
4345

46+
static llvm::cl::opt<bool> EnableSinkingOwnedForwardingInstToUses(
47+
"silcombine-owned-code-sinking",
48+
llvm::cl::desc("Enable sinking of owened forwarding insts"),
49+
llvm::cl::init(true), llvm::cl::Hidden);
50+
4451
//===----------------------------------------------------------------------===//
4552
// Utility Methods
4653
//===----------------------------------------------------------------------===//
@@ -145,6 +152,63 @@ class SILCombineCanonicalize final : CanonicalizeInstruction {
145152
}
146153
};
147154

155+
bool SILCombiner::trySinkOwnedForwardingInst(SingleValueInstruction *svi) {
156+
if (auto *consumingUse = svi->getSingleConsumingUse()) {
157+
auto *consumingUser = consumingUse->getUser();
158+
159+
// If our user is already in the same block, we don't move it further.
160+
if (svi->getParent() == consumingUser->getParent())
161+
return false;
162+
163+
// Otherwise, make sure our instruction does not have any non-debug uses
164+
// that are non-lifetime ending. If so, we return.
165+
if (llvm::any_of(getNonDebugUses(svi),
166+
[](Operand *use) { return !use->isLifetimeEnding(); }))
167+
return false;
168+
169+
// Otherwise, delete all of the debug uses so we don't have to sink them as
170+
// well and then return true so we process svi in its new position.
171+
deleteAllDebugUses(svi, instModCallbacks);
172+
svi->moveBefore(consumingUser);
173+
MadeChange = true;
174+
175+
// NOTE: We return nullptr here so that our caller doesn't delete the
176+
// instruction and instead tries to simplify it.
177+
return false;
178+
}
179+
180+
// If we have multiple consuming uses, then we know that our
181+
// forwarding inst must be live out of the current block and thus we
182+
// might be able to duplicate/sink.
183+
if (llvm::any_of(getNonDebugUses(svi),
184+
[](Operand *use) { return !use->isLifetimeEnding(); }))
185+
return false;
186+
187+
while (!svi->use_empty()) {
188+
auto *sviUse = *svi->use_begin();
189+
auto *sviUser = sviUse->getUser();
190+
191+
if (auto *dvi = dyn_cast<DestroyValueInst>(sviUser)) {
192+
dvi->setOperand(svi->getOperand(0));
193+
Worklist.add(dvi);
194+
continue;
195+
}
196+
197+
if (sviUser->isDebugInstruction()) {
198+
eraseInstFromFunction(*sviUser);
199+
continue;
200+
}
201+
202+
auto *newSVI = svi->clone(sviUser);
203+
Worklist.add(newSVI);
204+
sviUse->set(newSVI);
205+
}
206+
207+
eraseInstFromFunction(*svi);
208+
MadeChange = true;
209+
return true;
210+
}
211+
148212
bool SILCombiner::doOneIteration(SILFunction &F, unsigned Iteration) {
149213
MadeChange = false;
150214

@@ -184,7 +248,31 @@ bool SILCombiner::doOneIteration(SILFunction &F, unsigned Iteration) {
184248
}
185249

186250
// If we have reached this point, all attempts to do simple simplifications
187-
// have failed. Prepare to SILCombine.
251+
// have failed. First if we have an owned forwarding value, we try to
252+
// sink. Otherwise, we perform the actual SILCombine operation.
253+
if (EnableSinkingOwnedForwardingInstToUses) {
254+
// If we have an ownership forwarding single value inst that forwards
255+
// through its first argument and it is trivially duplicatable, see if it
256+
// only has consuming uses. If so, we can duplicate the instruction into
257+
// the consuming use blocks and destroy any destroy_value uses of it that
258+
// we see. This makes it easier for SILCombine to fold instructions with
259+
// owned paramaters since chains of these values will be in the same
260+
// block.
261+
if (auto *svi = dyn_cast<SingleValueInstruction>(I)) {
262+
if ((isa<FirstArgOwnershipForwardingSingleValueInst>(svi) ||
263+
isa<OwnershipForwardingConversionInst>(svi)) &&
264+
SILValue(svi).getOwnershipKind() == OwnershipKind::Owned) {
265+
// Try to sink the value. If we sank the value and deleted it,
266+
// continue. If we didn't optimize or sank but we are still able to
267+
// optimize further, we fall through to SILCombine below.
268+
if (trySinkOwnedForwardingInst(svi)) {
269+
continue;
270+
}
271+
}
272+
}
273+
}
274+
275+
// Then begin... SILCombine.
188276
Builder.setInsertionPoint(I);
189277

190278
#ifndef NDEBUG

lib/SILOptimizer/SILCombiner/SILCombiner.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,12 @@ class SILCombiner :
333333
bool tryOptimizeKeypathKVCString(ApplyInst *AI, FuncDecl *calleeFn,
334334
KeyPathInst *kp);
335335

336+
/// Sinks owned forwarding instructions to their uses if they do not have
337+
/// non-debug non-consuming uses. Deletes any debug_values and destroy_values
338+
/// when this is done. Returns true if we deleted svi and thus we should not
339+
/// try to visit it.
340+
bool trySinkOwnedForwardingInst(SingleValueInstruction *svi);
341+
336342
// Optimize concatenation of string literals.
337343
// Constant-fold concatenation of string literals known at compile-time.
338344
SILInstruction *optimizeConcatenationOfStringLiterals(ApplyInst *AI);

test/SILOptimizer/sil_combine_ossa.sil

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import Builtin
99
import Swift
1010

1111
class Klass {}
12+
13+
sil @use_klass_guaranteed : $@convention(thin) (@guaranteed Klass) -> ()
14+
1215
class RawBuffer {}
1316
class HeapBufferStorage<T, U> : RawBuffer {}
1417

@@ -47,6 +50,8 @@ struct MyInt {
4750
sil [global_init] @global_init_fun : $@convention(thin) () -> Builtin.RawPointer
4851

4952
sil [ossa] @user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
53+
sil [ossa] @use_nativeobject_owned : $@convention(thin) (@owned Builtin.NativeObject) -> ()
54+
sil [ossa] @use_nativeobject_guaranteed : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
5055

5156
sil [ossa] @unknown : $@convention(thin) () -> ()
5257

@@ -4111,3 +4116,97 @@ bb0(%0 : @owned $Klass):
41114116
destroy_value %0 : $Klass
41124117
return %2 : $Optional<Klass>
41134118
}
4119+
4120+
// We do not sink if the forwarding inst is in the same block as the consuming use.
4121+
//
4122+
// CHECK-LABEL: sil [ossa] @sink_forwarding_owned_uses_no_sink_same_block : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4123+
// CHECK: bb0(
4124+
// CHECK-NEXT: unchecked_ref_cast
4125+
// CHECK-NEXT: function_ref
4126+
// CHECK: } // end sil function 'sink_forwarding_owned_uses_no_sink_same_block'
4127+
sil [ossa] @sink_forwarding_owned_uses_no_sink_same_block : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4128+
bb0(%0a : @owned $Klass, %0b : @guaranteed $Klass):
4129+
%1 = unchecked_ref_cast %0a : $Klass to $Builtin.NativeObject
4130+
%2 = function_ref @use_klass_guaranteed : $@convention(thin) (@guaranteed Klass) -> ()
4131+
apply %2(%0b) : $@convention(thin) (@guaranteed Klass) -> ()
4132+
return %1 : $Builtin.NativeObject
4133+
}
4134+
4135+
// In this case, since we have different blocks, we sink.
4136+
//
4137+
// CHECK-LABEL: sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4138+
// CHECK: bb0(
4139+
// CHECK-NEXT: br bb1
4140+
// CHECK: bb1:
4141+
// CHECK-NEXT: unchecked_ref_cast
4142+
// CHECK-NEXT: return
4143+
// CHECK: } // end sil function 'sink_forwarding_owned_uses_sink_diff_block_single_use'
4144+
sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4145+
bb0(%0a : @owned $Klass, %0b : @guaranteed $Klass):
4146+
%1 = unchecked_ref_cast %0a : $Klass to $Builtin.NativeObject
4147+
br bb1
4148+
4149+
bb1:
4150+
return %1 : $Builtin.NativeObject
4151+
}
4152+
4153+
// CHECK-LABEL: sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use_noopt : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4154+
// CHECK: bb0(
4155+
// CHECK-NEXT: unchecked_ref_cast
4156+
// CHECK-NEXT: function_ref
4157+
// CHECK-NEXT: function_ref
4158+
// CHECK-NEXT: apply
4159+
// CHECK-NEXT: br bb1
4160+
// CHECK: bb1:
4161+
// CHECK-NEXT: return
4162+
// CHECK: } // end sil function 'sink_forwarding_owned_uses_sink_diff_block_single_use_noopt'
4163+
sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use_noopt : $@convention(thin) (@owned Klass, @guaranteed Klass) -> @owned Builtin.NativeObject {
4164+
bb0(%0a : @owned $Klass, %0b : @guaranteed $Klass):
4165+
%1 = unchecked_ref_cast %0a : $Klass to $Builtin.NativeObject
4166+
%f = function_ref @use_nativeobject_guaranteed : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
4167+
apply %f(%1) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
4168+
br bb1
4169+
4170+
bb1:
4171+
return %1 : $Builtin.NativeObject
4172+
}
4173+
4174+
// In this case, since we have different blocks, we sink.
4175+
//
4176+
// CHECK-LABEL: sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use_destroy : $@convention(thin) (@owned Klass, @guaranteed Klass) -> () {
4177+
// CHECK: bb0(
4178+
// CHECK-NEXT: cond_br undef, bb1, bb2
4179+
//
4180+
// CHECK: bb1:
4181+
// CHECK-NEXT: function_ref
4182+
// CHECK-NEXT: function_ref
4183+
// CHECK-NEXT: unchecked_ref_cast
4184+
// CHECK-NEXT: apply
4185+
// CHECK-NEXT: br bb3
4186+
//
4187+
// CHECK: bb2:
4188+
// CHECK-NEXT: destroy_value
4189+
// CHECK-NEXT: br bb3
4190+
//
4191+
// CHECK: bb3:
4192+
// CHECK-NEXT: tuple
4193+
// CHECK-NEXT: return
4194+
// CHECK: } // end sil function 'sink_forwarding_owned_uses_sink_diff_block_single_use_destroy'
4195+
sil [ossa] @sink_forwarding_owned_uses_sink_diff_block_single_use_destroy : $@convention(thin) (@owned Klass, @guaranteed Klass) -> () {
4196+
bb0(%0a : @owned $Klass, %0b : @guaranteed $Klass):
4197+
%1 = unchecked_ref_cast %0a : $Klass to $Builtin.NativeObject
4198+
cond_br undef, bb1a, bb1b
4199+
4200+
bb1a:
4201+
%f = function_ref @use_nativeobject_owned : $@convention(thin) (@owned Builtin.NativeObject) -> ()
4202+
apply %f(%1) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
4203+
br bb1
4204+
4205+
bb1b:
4206+
destroy_value %1 : $Builtin.NativeObject
4207+
br bb1
4208+
4209+
bb1:
4210+
%9999 = tuple()
4211+
return %9999 : $()
4212+
}

0 commit comments

Comments
 (0)