Skip to content

Commit 8dbac48

Browse files
meg-guptameg-gupta
authored
Merge pull request #33205 from meg-gupta/ometofunctionpass
[ownership] Convert OME into a function pass
2 parents c22ba90 + f5baf7a commit 8dbac48

File tree

9 files changed

+100
-68
lines changed

9 files changed

+100
-68
lines changed

‎include/swift/SIL/SILCloner.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,13 @@ template <typename ImplClass>
15421542
void SILCloner<ImplClass>::visitUncheckedValueCastInst(
15431543
UncheckedValueCastInst *Inst) {
15441544
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
1545+
if (!getBuilder().hasOwnership()) {
1546+
recordClonedInstruction(Inst, getBuilder().createUncheckedBitwiseCast(
1547+
getOpLocation(Inst->getLoc()),
1548+
getOpValue(Inst->getOperand()),
1549+
getOpType(Inst->getType())));
1550+
return;
1551+
}
15451552
recordClonedInstruction(Inst, getBuilder().createUncheckedValueCast(
15461553
getOpLocation(Inst->getLoc()),
15471554
getOpValue(Inst->getOperand()),

‎include/swift/SIL/SILModule.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,15 @@ class SILModule {
263263
/// to ensure that the module is serialized only once.
264264
bool serialized;
265265

266+
/// Set if we have registered a deserialization notification handler for
267+
/// lowering ownership in non transparent functions.
268+
/// This gets set in NonTransparent OwnershipModelEliminator pass.
269+
bool regDeserializationNotificationHandlerForNonTransparentFuncOME;
270+
/// Set if we have registered a deserialization notification handler for
271+
/// lowering ownership in transparent functions.
272+
/// This gets set in OwnershipModelEliminator pass.
273+
bool regDeserializationNotificationHandlerForAllFuncOME;
274+
266275
/// Action to be executed for serializing the SILModule.
267276
ActionCallback SerializeSILAction;
268277

@@ -301,6 +310,19 @@ class SILModule {
301310
deserializationNotificationHandlers.erase(handler);
302311
}
303312

313+
bool hasRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME() {
314+
return regDeserializationNotificationHandlerForNonTransparentFuncOME;
315+
}
316+
bool hasRegisteredDeserializationNotificationHandlerForAllFuncOME() {
317+
return regDeserializationNotificationHandlerForAllFuncOME;
318+
}
319+
void setRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME() {
320+
regDeserializationNotificationHandlerForNonTransparentFuncOME = true;
321+
}
322+
void setRegisteredDeserializationNotificationHandlerForAllFuncOME() {
323+
regDeserializationNotificationHandlerForAllFuncOME = true;
324+
}
325+
304326
/// Add a delete notification handler \p Handler to the module context.
305327
void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler);
306328

‎include/swift/SILOptimizer/PassManager/PassManager.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,11 @@ class SILPassManager {
281281
/// Run the passes in Transform from \p FromTransIdx to \p ToTransIdx.
282282
void runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx);
283283

284+
/// Helper function to check if the function pass should be run mandatorily
285+
/// All passes in mandatory pass pipeline and ownership model elimination are
286+
/// mandatory function passes.
287+
bool isMandatoryFunctionPass(SILFunctionTransform *);
288+
284289
/// A helper function that returns (based on SIL stage and debug
285290
/// options) whether we should continue running passes.
286291
bool continueTransforming();

‎lib/SIL/IR/SILModule.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ class SILModule::SerializationCallback final
9595
SILModule::SILModule(llvm::PointerUnion<FileUnit *, ModuleDecl *> context,
9696
Lowering::TypeConverter &TC, const SILOptions &Options)
9797
: Stage(SILStage::Raw), Options(Options), serialized(false),
98+
regDeserializationNotificationHandlerForNonTransparentFuncOME(false),
99+
regDeserializationNotificationHandlerForAllFuncOME(false),
98100
SerializeSILAction(), Types(TC) {
99101
assert(!context.isNull());
100102
if (auto *file = context.dyn_cast<FileUnit *>()) {

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ static void prepareSILFunctionForOptimization(ModuleDecl *, SILFunction *F) {
370370

371371
namespace {
372372

373-
struct OwnershipModelEliminator : SILModuleTransform {
373+
struct OwnershipModelEliminator : SILFunctionTransform {
374374
bool SkipTransparent;
375375
bool SkipStdlibModule;
376376

@@ -379,53 +379,50 @@ struct OwnershipModelEliminator : SILModuleTransform {
379379

380380
void run() override {
381381
if (DumpBefore.size()) {
382-
getModule()->dump(DumpBefore.c_str());
382+
getFunction()->dump(DumpBefore.c_str());
383383
}
384384

385-
auto &Mod = *getModule();
385+
auto *F = getFunction();
386+
auto &Mod = getFunction()->getModule();
386387

387388
// If we are supposed to skip the stdlib module and we are in the stdlib
388389
// module bail.
389390
if (SkipStdlibModule && Mod.isStdlibModule()) {
390391
return;
391392
}
392393

393-
for (auto &F : Mod) {
394-
// If F does not have ownership, skip it. We have no further work to do.
395-
if (!F.hasOwnership())
396-
continue;
397-
398-
// If we were asked to not strip ownership from transparent functions in
399-
// /our/ module, continue.
400-
if (SkipTransparent && F.isTransparent())
401-
continue;
402-
403-
// Verify here to make sure ownership is correct before we strip.
404-
{
405-
// Add a pretty stack trace entry to tell users who see a verification
406-
// failure triggered by this verification check that they need to re-run
407-
// with -sil-verify-all to actually find the pass that introduced the
408-
// verification error.
409-
//
410-
// DISCUSSION: This occurs due to the crash from the verification
411-
// failure happening in the pass itself. This causes us to dump the
412-
// SILFunction and emit a msg that this pass (OME) is the culprit. This
413-
// is generally correct for most passes, but not for OME since we are
414-
// verifying before we have even modified the function to ensure that
415-
// all ownership invariants have been respected before we lower
416-
// ownership from the function.
417-
llvm::PrettyStackTraceString silVerifyAllMsgOnFailure(
418-
"Found verification error when verifying before lowering "
419-
"ownership. Please re-run with -sil-verify-all to identify the "
420-
"actual pass that introduced the verification error.");
421-
F.verify();
422-
}
394+
if (!F->hasOwnership())
395+
return;
423396

424-
if (stripOwnership(F)) {
425-
auto InvalidKind =
426-
SILAnalysis::InvalidationKind::BranchesAndInstructions;
427-
invalidateAnalysis(&F, InvalidKind);
428-
}
397+
// If we were asked to not strip ownership from transparent functions in
398+
// /our/ module, return.
399+
if (SkipTransparent && F->isTransparent())
400+
return;
401+
402+
// Verify here to make sure ownership is correct before we strip.
403+
{
404+
// Add a pretty stack trace entry to tell users who see a verification
405+
// failure triggered by this verification check that they need to re-run
406+
// with -sil-verify-all to actually find the pass that introduced the
407+
// verification error.
408+
//
409+
// DISCUSSION: This occurs due to the crash from the verification
410+
// failure happening in the pass itself. This causes us to dump the
411+
// SILFunction and emit a msg that this pass (OME) is the culprit. This
412+
// is generally correct for most passes, but not for OME since we are
413+
// verifying before we have even modified the function to ensure that
414+
// all ownership invariants have been respected before we lower
415+
// ownership from the function.
416+
llvm::PrettyStackTraceString silVerifyAllMsgOnFailure(
417+
"Found verification error when verifying before lowering "
418+
"ownership. Please re-run with -sil-verify-all to identify the "
419+
"actual pass that introduced the verification error.");
420+
F->verify();
421+
}
422+
423+
if (stripOwnership(*F)) {
424+
auto InvalidKind = SILAnalysis::InvalidationKind::BranchesAndInstructions;
425+
invalidateAnalysis(InvalidKind);
429426
}
430427

431428
// If we were asked to strip transparent, we are at the beginning of the
@@ -435,12 +432,19 @@ struct OwnershipModelEliminator : SILModuleTransform {
435432
FunctionBodyDeserializationNotificationHandler;
436433
std::unique_ptr<DeserializationNotificationHandler> ptr;
437434
if (SkipTransparent) {
438-
ptr.reset(new NotificationHandlerTy(
439-
prepareNonTransparentSILFunctionForOptimization));
435+
if (!Mod.hasRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME()) {
436+
ptr.reset(new NotificationHandlerTy(
437+
prepareNonTransparentSILFunctionForOptimization));
438+
Mod.registerDeserializationNotificationHandler(std::move(ptr));
439+
Mod.setRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME();
440+
}
440441
} else {
441-
ptr.reset(new NotificationHandlerTy(prepareSILFunctionForOptimization));
442+
if (!Mod.hasRegisteredDeserializationNotificationHandlerForAllFuncOME()) {
443+
ptr.reset(new NotificationHandlerTy(prepareSILFunctionForOptimization));
444+
Mod.registerDeserializationNotificationHandler(std::move(ptr));
445+
Mod.setRegisteredDeserializationNotificationHandlerForAllFuncOME();
446+
}
442447
}
443-
Mod.registerDeserializationNotificationHandler(std::move(ptr));
444448
}
445449
};
446450

‎lib/SILOptimizer/PassManager/PassManager.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,21 +383,35 @@ void SILPassManager::dumpPassInfo(const char *Title, unsigned TransIdx,
383383
llvm::dbgs() << '\n';
384384
}
385385

386+
bool SILPassManager::isMandatoryFunctionPass(SILFunctionTransform *sft) {
387+
return isMandatory || sft->getPassKind() ==
388+
PassKind::NonTransparentFunctionOwnershipModelEliminator ||
389+
sft->getPassKind() == PassKind::OwnershipModelEliminator ||
390+
sft->getPassKind() ==
391+
PassKind::NonStdlibNonTransparentFunctionOwnershipModelEliminator;
392+
}
393+
386394
void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
387395

388396
assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
389397

390398
auto *SFT = cast<SILFunctionTransform>(Transformations[TransIdx]);
399+
400+
if (!F->shouldOptimize() && !isMandatoryFunctionPass(SFT)) {
401+
return;
402+
}
403+
391404
SFT->injectPassManager(this);
392405
SFT->injectFunction(F);
393406

394407
PrettyStackTraceSILFunctionTransform X(SFT, NumPassesRun);
395408
DebugPrintEnabler DebugPrint(NumPassesRun);
396409

397410
// If nothing changed since the last run of this pass, we can skip this
398-
// pass.
411+
// pass if it is not mandatory
399412
CompletedPasses &completedPasses = CompletedPassesMap[F];
400-
if (completedPasses.test((size_t)SFT->getPassKind()) &&
413+
if (!isMandatoryFunctionPass(SFT) &&
414+
completedPasses.test((size_t)SFT->getPassKind()) &&
401415
!SILDisableSkippingPasses) {
402416
if (SILPrintPassName)
403417
dumpPassInfo("(Skip)", TransIdx, F);
@@ -513,7 +527,7 @@ runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx) {
513527

514528
// Only include functions that are definitions, and which have not
515529
// been intentionally excluded from optimization.
516-
if (F.isDefinition() && (isMandatory || F.shouldOptimize()))
530+
if (F.isDefinition())
517531
FunctionWorklist.push_back(*I);
518532
}
519533

‎test/DebugInfo/dbgvalue-insertpt.swift

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,9 @@
1-
// RUN: %target-swift-frontend -g -emit-ir %s | %FileCheck %s
1+
// RUN: %target-swift-frontend -g -emit-ir -Xllvm '-sil-inline-never-functions=next' %s | %FileCheck %s
22
// FIXME: This test should be testing a non-shadow-copied value instead.
33
for i in 0 ..< 3 {
44
// CHECK: %[[ALLOCA:[0-9]+]] = alloca %TSiSg
55
// CHECK: %i.debug = alloca i{{32|64}}
66
// CHECK-NEXT: call void @llvm.dbg.declare(metadata i{{32|64}}* %i.debug,
77
// CHECK-SAME: metadata ![[I:[0-9]+]],
8-
// CHECK: call swiftcc{{.*}} @{{.*}}next{{.*}}
9-
// CHECK: %[[LD:[0-9]+]] = load i{{32|64}}, i{{32|64}}*
10-
// CHECK: br i1 {{%.*}}, label %[[FAIL:.*]], label %[[SUCCESS:.*]],
11-
//
12-
// CHECK: [[SUCCESS]]:
13-
// CHECK: br label %[[NEXT_BB:.*]],
14-
//
15-
// CHECK: [[NEXT_BB]]:
16-
// CHECK: %[[PHI_VAL:.*]] = phi i{{32|64}} [ %[[LD]], %[[SUCCESS]] ]
17-
// CHECK: store i{{32|64}} %[[PHI_VAL]], i{{32|64}}* %i.debug
188
// CHECK: ![[I]] = !DILocalVariable(name: "i",
199
}

‎test/IRGen/big_types_corner_cases.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,6 @@ public func testGetFunc() {
204204
// CHECK: [[CALL1:%.*]] = call {{.*}} @__swift_instantiateConcreteTypeFromMangledName({{.*}} @"$sSayy22big_types_corner_cases9BigStructVcSgGMD"
205205
// CHECK: [[CALL2:%.*]] = call i8** @"$sSayy22big_types_corner_cases9BigStructVcSgGSayxGSlsWl
206206
// CHECK: call swiftcc void @"$sSlsE10firstIndex5where0B0QzSgSb7ElementQzKXE_tKF"(%TSq.{{.*}}* noalias nocapture sret %{{[0-9]+}}, i8* bitcast ({{.*}}* @"$s22big_types_corner_cases9BigStruct{{.*}}_TRTA{{(\.ptrauth)?}}" to i8*), %swift.opaque* %{{[0-9]+}}, %swift.type* %{{[0-9]+}}, i8** [[CALL2]]
207-
208-
// CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} hidden swiftcc void @"$s22big_types_corner_cases7TestBigC5test2yyF"(%T22big_types_corner_cases7TestBigC* swiftself %0)
209-
// CHECK: [[CALL1:%.*]] = call {{.*}} @__swift_instantiateConcreteTypeFromMangledName({{.*}} @"$sSaySS2ID_y22big_types_corner_cases9BigStructVcSg7handlertGMD"
210-
// CHECK: [[CALL2:%.*]] = call i8** @"$sSaySS2ID_y22big_types_corner_cases9BigStructVcSg7handlertGSayxGSlsWl"
211-
// CHECK: call swiftcc void @"$sSlss16IndexingIteratorVyxG0B0RtzrlE04makeB0ACyF"(%Ts16IndexingIteratorV{{.*}}* noalias nocapture sret {{.*}}, %swift.type* [[CALL1]], i8** [[CALL2]], %swift.opaque* noalias nocapture swiftself {{.*}})
212-
// CHECK: ret void
213207
class TestBig {
214208
typealias Handler = (BigStruct) -> Void
215209

‎test/SILOptimizer/prespecialize.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,6 @@
77

88
// CHECK-LABEL: sil [noinline] @$s13prespecialize4test_4sizeySaySiGz_SitF
99
//
10-
// function_ref specialized Collection<A where ...>.makeIterator() -> IndexingIterator<A>
11-
// CHECK: function_ref @$sSlss16IndexingIteratorVyxG0B0RtzrlE04makeB0ACyFSnySiG_Tg5
12-
//
13-
// function_ref specialized IndexingIterator.next() -> A.Element?
14-
// CHECK: function_ref @$ss16IndexingIteratorV4next7ElementQzSgyFSnySiG_Tg5
15-
//
1610
// Look for generic specialization <Swift.Int> of Swift.Array.subscript.getter : (Swift.Int) -> A
1711
// CHECK: function_ref @$sSayxSicigSi_Tg5
1812
// CHECK: return

0 commit comments

Comments
 (0)