Skip to content

Commit e670d2b

Browse files
gottesmmgottesmm
committed
[sil-combine] Canonicalize copies using canonicalizeOSSALifetimes when moving newly created instructions into the worklist.
To be more explicit, canonicalizeOSSALifetimes is a utility that re-canonicalizes all at once a set of defs that the caller found by applying CanonicalizeOSSALifetime::getCanonicalCopiedDef(copy)). The reason why I am doing this is that when we RAUW in OSSA, we sometimes insert additional copies to make the problem easier for a utility to handle. This lets us canonicalize away any copies before we even leave the pass.
1 parent 387806b commit e670d2b

File tree

3 files changed

+88
-49
lines changed

3 files changed

+88
-49
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombine.cpp‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@
2121
#define DEBUG_TYPE "sil-combine"
2222

2323
#include "SILCombiner.h"
24+
#include "swift/SIL/BasicBlockDatastructures.h"
2425
#include "swift/SIL/DebugUtils.h"
2526
#include "swift/SIL/SILBuilder.h"
2627
#include "swift/SIL/SILVisitor.h"
27-
#include "swift/SIL/BasicBlockDatastructures.h"
2828
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
29+
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
30+
#include "swift/SILOptimizer/Analysis/NonLocalAccessBlockAnalysis.h"
2931
#include "swift/SILOptimizer/Analysis/SimplifyInstruction.h"
3032
#include "swift/SILOptimizer/PassManager/Passes.h"
3133
#include "swift/SILOptimizer/PassManager/Transforms.h"
34+
#include "swift/SILOptimizer/Utils/CanonicalOSSALifetime.h"
3235
#include "swift/SILOptimizer/Utils/CanonicalizeInstruction.h"
3336
#include "swift/SILOptimizer/Utils/DebugOptUtils.h"
3437
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
@@ -299,10 +302,39 @@ bool SILCombiner::doOneIteration(SILFunction &F, unsigned Iteration) {
299302
}
300303

301304
// Our tracking list has been accumulating instructions created by the
302-
// SILBuilder during this iteration. Go through the tracking list and add
303-
// its contents to the worklist and then clear said list in preparation for
304-
// the next iteration.
305+
// SILBuilder during this iteration. In order to finish this round of
306+
// SILCombine, go through the tracking list and add its contents to the
307+
// worklist and then clear said list in preparation for the next
308+
// iteration. We canonicalize any copies that we created in order to
309+
// eliminate unnecessary copies introduced by RAUWing when ownership is
310+
// enabled.
311+
//
312+
// NOTE: It is ok if copy propagation results in MadeChanges being set to
313+
// true. This is because we only add elements to the tracking list if we
314+
// actually made a change to the IR, so MadeChanges should already be true
315+
// at this point.
305316
auto &TrackingList = *Builder.getTrackingList();
317+
if (TrackingList.size() && Builder.hasOwnership()) {
318+
SmallSetVector<SILValue, 16> defsToCanonicalize;
319+
for (auto *trackedInst : TrackingList) {
320+
if (auto *cvi = dyn_cast<CopyValueInst>(trackedInst)) {
321+
defsToCanonicalize.insert(
322+
CanonicalizeOSSALifetime::getCanonicalCopiedDef(cvi));
323+
}
324+
}
325+
if (defsToCanonicalize.size()) {
326+
CanonicalizeOSSALifetime canonicalizer(
327+
false /*prune debug*/, false /*canonicalize borrows*/,
328+
false /*poison refs*/, NLABA, DA, instModCallbacks);
329+
auto analysisInvalidation = canonicalizeOSSALifetimes(
330+
canonicalizer, defsToCanonicalize.getArrayRef());
331+
if (bool(analysisInvalidation)) {
332+
NLABA->lockInvalidation();
333+
parentTransform->invalidateAnalysis(analysisInvalidation);
334+
NLABA->unlockInvalidation();
335+
}
336+
}
337+
}
306338
for (SILInstruction *I : TrackingList) {
307339
LLVM_DEBUG(llvm::dbgs() << "SC: add " << *I
308340
<< " from tracking list to worklist\n");
@@ -359,12 +391,13 @@ class SILCombine : public SILFunctionTransform {
359391
auto *DA = PM->getAnalysis<DominanceAnalysis>();
360392
auto *PCA = PM->getAnalysis<ProtocolConformanceAnalysis>();
361393
auto *CHA = PM->getAnalysis<ClassHierarchyAnalysis>();
394+
auto *NLABA = PM->getAnalysis<NonLocalAccessBlockAnalysis>();
362395

363396
SILOptFunctionBuilder FuncBuilder(*this);
364397
// Create a SILBuilder with a tracking list for newly added
365398
// instructions, which we will periodically move to our worklist.
366399
SILBuilder B(*getFunction(), &TrackingList);
367-
SILCombiner Combiner(FuncBuilder, B, AA, DA, PCA, CHA,
400+
SILCombiner Combiner(this, FuncBuilder, B, AA, DA, PCA, CHA, NLABA,
368401
getOptions().RemoveRuntimeAsserts);
369402
bool Changed = Combiner.runOnFunction(*getFunction());
370403
assert(TrackingList.empty() &&

‎lib/SILOptimizer/SILCombiner/SILCombiner.h‎

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/SIL/SILValue.h"
3030
#include "swift/SIL/SILVisitor.h"
3131
#include "swift/SILOptimizer/Analysis/ClassHierarchyAnalysis.h"
32+
#include "swift/SILOptimizer/Analysis/NonLocalAccessBlockAnalysis.h"
3233
#include "swift/SILOptimizer/Analysis/ProtocolConformanceAnalysis.h"
3334
#include "swift/SILOptimizer/Utils/CastOptimizer.h"
3435
#include "swift/SILOptimizer/Utils/Existential.h"
@@ -46,6 +47,7 @@ class AliasAnalysis;
4647
/// the worklist.
4748
class SILCombiner :
4849
public SILInstructionVisitor<SILCombiner, SILInstruction *> {
50+
SILFunctionTransform *parentTransform;
4951

5052
AliasAnalysis *AA;
5153

@@ -59,6 +61,10 @@ class SILCombiner :
5961
/// conforming class.
6062
ClassHierarchyAnalysis *CHA;
6163

64+
/// Non local access block analysis that we use when canonicalize object
65+
/// lifetimes in OSSA.
66+
NonLocalAccessBlockAnalysis *NLABA;
67+
6268
/// Worklist containing all of the instructions primed for simplification.
6369
SmallSILInstructionWorklist<256> Worklist;
6470

@@ -97,39 +103,47 @@ class SILCombiner :
97103
OwnershipFixupContext ownershipFixupContext;
98104

99105
public:
100-
SILCombiner(SILOptFunctionBuilder &FuncBuilder, SILBuilder &B,
106+
SILCombiner(SILFunctionTransform *parentTransform,
107+
SILOptFunctionBuilder &FuncBuilder, SILBuilder &B,
101108
AliasAnalysis *AA, DominanceAnalysis *DA,
102109
ProtocolConformanceAnalysis *PCA, ClassHierarchyAnalysis *CHA,
103-
bool removeCondFails)
104-
: AA(AA), DA(DA), PCA(PCA), CHA(CHA), Worklist("SC"),
105-
deadEndBlocks(&B.getFunction()), MadeChange(false),
106-
RemoveCondFails(removeCondFails), Iteration(0), Builder(B),
107-
CastOpt(
108-
FuncBuilder, nullptr /*SILBuilderContext*/,
109-
/* ReplaceValueUsesAction */
110-
[&](SILValue Original, SILValue Replacement) {
111-
replaceValueUsesWith(Original, Replacement);
112-
},
113-
/* ReplaceInstUsesAction */
114-
[&](SingleValueInstruction *I, ValueBase *V) {
115-
replaceInstUsesWith(*I, V);
116-
},
117-
/* EraseAction */
118-
[&](SILInstruction *I) { eraseInstFromFunction(*I); }),
119-
instModCallbacks(),
120-
deBlocks(&B.getFunction()),
110+
NonLocalAccessBlockAnalysis *NLABA, bool removeCondFails)
111+
: parentTransform(parentTransform), AA(AA), DA(DA), PCA(PCA), CHA(CHA),
112+
NLABA(NLABA), Worklist("SC"), deadEndBlocks(&B.getFunction()),
113+
MadeChange(false), RemoveCondFails(removeCondFails), Iteration(0),
114+
Builder(B), CastOpt(
115+
FuncBuilder, nullptr /*SILBuilderContext*/,
116+
/* ReplaceValueUsesAction */
117+
[&](SILValue Original, SILValue Replacement) {
118+
replaceValueUsesWith(Original, Replacement);
119+
},
120+
/* ReplaceInstUsesAction */
121+
[&](SingleValueInstruction *I, ValueBase *V) {
122+
replaceInstUsesWith(*I, V);
123+
},
124+
/* EraseAction */
125+
[&](SILInstruction *I) { eraseInstFromFunction(*I); }),
126+
instModCallbacks(), deBlocks(&B.getFunction()),
121127
ownershipFixupContext(instModCallbacks, deBlocks) {
122-
instModCallbacks = InstModCallbacks()
123-
.onDelete([&](SILInstruction *instToDelete) {
124-
eraseInstFromFunction(*instToDelete);
125-
})
126-
.onCreateNewInst([&](SILInstruction *newlyCreatedInst) {
127-
Worklist.add(newlyCreatedInst);
128-
})
129-
.onSetUseValue([&](Operand *use, SILValue newValue) {
130-
use->set(newValue);
131-
Worklist.add(use->getUser());
132-
});
128+
instModCallbacks =
129+
InstModCallbacks()
130+
.onDelete([&](SILInstruction *instToDelete) {
131+
// We allow for users in SILCombine to perform 2 stage deletion,
132+
// so we need to split the erasing of instructions from adding
133+
// operands to the worklist.
134+
eraseInstFromFunction(
135+
*instToDelete, false /*do not add operands to the worklist*/);
136+
})
137+
.onNotifyWillBeDeleted([&](SILInstruction *instThatWillBeDeleted) {
138+
Worklist.addOperandsToWorklist(*instThatWillBeDeleted);
139+
})
140+
.onCreateNewInst([&](SILInstruction *newlyCreatedInst) {
141+
Worklist.add(newlyCreatedInst);
142+
})
143+
.onSetUseValue([&](Operand *use, SILValue newValue) {
144+
use->set(newValue);
145+
Worklist.add(use->getUser());
146+
});
133147
}
134148

135149
bool runOnFunction(SILFunction &F);

‎test/SILOptimizer/sil_combine_apply_ossa.sil‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,10 @@ sil [ossa] @guaranteed_closure : $@convention(thin) (@guaranteed C) -> ()
518518
// CHECK: bb0([[ARG:%.*]] : @guaranteed $C):
519519
// CHECK: [[ARG_COPY:%.*]] = copy_value [[ARG]]
520520
// CHECK: [[F:%.*]] = function_ref @guaranteed_closure
521-
// CHECK: [[ARG_COPY_2:%.*]] = copy_value [[ARG_COPY]]
522521
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [[F]]([[ARG_COPY]])
523-
// CHECK: apply [[F]]([[ARG_COPY_2]])
522+
// CHECK: apply [[F]]([[ARG]])
524523
// Don't release the closure context -- it is @callee_guaranteed.
525524
// CHECK-NOT: destroy_value [[C]] : $@callee_guaranteed () -> ()
526-
// CHECK: destroy_value [[ARG_COPY_2]]
527525
// CHECK-NOT: destroy_value [[C]] : $@callee_guaranteed () -> ()
528526
// CHECK: return [[C]]
529527
// CHECK: } // end sil function 'test_guaranteed_closure'
@@ -942,18 +940,14 @@ bb3:
942940
// CHECK-LABEL: sil [ossa] @test_apply_of_partial_apply_owned : $@convention(thin) (@in CC1) -> () {
943941
// CHECK: bb0([[ARG:%.*]] :
944942
// CHECK: [[LOAD_ARG:%.*]] = load [take] [[ARG]]
945-
// CHECK: [[LOAD_ARG_COPY:%.*]] = copy_value [[LOAD_ARG]]
946-
// CHECK: destroy_value [[LOAD_ARG]]
947943
// CHECK: cond_br undef, bb1, bb2
948944
//
949945
// CHECK: bb1:
950-
// CHECK: destroy_value [[LOAD_ARG_COPY]]
946+
// CHECK: destroy_value [[LOAD_ARG]]
951947
// CHECK: br bb3
952948
//
953949
// CHECK: bb2:
954-
// CHECK: [[LOAD_ARG_COPY_COPY:%.*]] = copy_value [[LOAD_ARG_COPY]]
955-
// CHECK: apply {{%.*}}([[LOAD_ARG_COPY_COPY]]) : $@convention(method) (@owned CC1) -> ()
956-
// CHECK: destroy_value [[LOAD_ARG_COPY]]
950+
// CHECK: apply {{%.*}}([[LOAD_ARG]]) : $@convention(method) (@owned CC1) -> ()
957951
// CHECK: br bb3
958952
//
959953
// CHECK: bb3:
@@ -981,17 +975,15 @@ bb3:
981975
// CHECK-LABEL: sil [ossa] @test_apply_of_partial_apply_guaranteed : $@convention(thin) (@in CC1) -> () {
982976
// CHECK: bb0([[ARG:%.*]] :
983977
// CHECK: [[LOAD_ARG:%.*]] = load [take] [[ARG]]
984-
// CHECK: [[LOAD_ARG_COPY:%.*]] = copy_value [[LOAD_ARG]]
985-
// CHECK: destroy_value [[LOAD_ARG]]
986978
// CHECK: cond_br undef, bb1, bb2
987979
//
988980
// CHECK: bb1:
989-
// CHECK: destroy_value [[LOAD_ARG_COPY]]
981+
// CHECK: destroy_value [[LOAD_ARG]]
990982
// CHECK: br bb3
991983
//
992984
// CHECK: bb2:
993-
// CHECK: apply {{%.*}}([[LOAD_ARG_COPY]]) : $@convention(method) (@guaranteed CC1) -> ()
994-
// CHECK: destroy_value [[LOAD_ARG_COPY]]
985+
// CHECK: apply {{%.*}}([[LOAD_ARG]]) : $@convention(method) (@guaranteed CC1) -> ()
986+
// CHECK: destroy_value [[LOAD_ARG]]
995987
// CHECK: br bb3
996988
//
997989
// CHECK: bb3:

0 commit comments

Comments
 (0)