Skip to content

Lower OwnershipModelEliminator to just before inlining #68180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,7 @@ private func isValidUseOfObject(_ use: Operand) -> Bool {
is UpcastInst,
is BeginDeallocRefInst,
is RefTailAddrInst,
is RefElementAddrInst,
is StructInst,
is PointerToAddressInst,
is IndexAddrInst:
is RefElementAddrInst:
for instUse in (inst as! SingleValueInstruction).uses {
if !isValidUseOfObject(instUse) {
return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,6 @@ private func shouldInline(apply: FullApplySite, callee: Function, alreadyInlined
return true
}

if apply.parentFunction.hasOwnership && !callee.hasOwnership {
// Cannot inline a non-ossa function into an ossa function
return false
}

if apply is BeginApplyInst {
// Avoid co-routines because they might allocate (their context).
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,14 @@ extension FullApplySite {
!calleeFunction.isSerialized {
return false
}

// Cannot inline a non-ossa function into an ossa function
if parentFunction.hasOwnership,
let calleeFunction = referencedFunction,
!calleeFunction.hasOwnership {
return false
}

return true
}

Expand Down
46 changes: 14 additions & 32 deletions lib/SILOptimizer/PassManager/PassPipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ static llvm::cl::opt<bool> SILViewSILGenCFG(
"sil-view-silgen-cfg", llvm::cl::init(false),
llvm::cl::desc("Enable the sil cfg viewer pass before diagnostics"));


llvm::cl::opt<bool> SILDisableLateOMEByDefault(
"sil-disable-late-ome-by-default", llvm::cl::init(false),
llvm::cl::desc(
"Disable late OME for non-transparent functions by default"));

llvm::cl::opt<bool>
EnableDestroyHoisting("enable-destroy-hoisting", llvm::cl::init(false),
llvm::cl::desc("Enable the DestroyHoisting pass."));
Expand Down Expand Up @@ -452,16 +446,6 @@ void addFunctionPasses(SILPassPipelinePlan &P,
// Cleanup, which is important if the inliner has restarted the pass pipeline.
P.addPerformanceConstantPropagation();

if (!P.getOptions().EnableOSSAModules && !SILDisableLateOMEByDefault) {
if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
return;

if (SILPrintFinalOSSAModule) {
addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
}
P.addNonTransparentFunctionOwnershipModelEliminator();
}

addSimplifyCFGSILCombinePasses(P);

P.addArrayElementPropagation();
Expand Down Expand Up @@ -506,13 +490,21 @@ void addFunctionPasses(SILPassPipelinePlan &P,
}
P.addARCSequenceOpts();

if (P.getOptions().EnableOSSAModules) {
// We earlier eliminated ownership if we are not compiling the stdlib. Now
// handle the stdlib functions, re-simplifying, eliminating ARC as we do.
if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
P.addCopyPropagation();
// We earlier eliminated ownership if we are not compiling the stdlib. Now
// handle the stdlib functions, re-simplifying, eliminating ARC as we do.
if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
P.addCopyPropagation();
}
P.addSemanticARCOpts();

if (!P.getOptions().EnableOSSAModules) {
if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
return;

if (SILPrintFinalOSSAModule) {
addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
}
P.addSemanticARCOpts();
P.addNonTransparentFunctionOwnershipModelEliminator();
}

switch (OpLevel) {
Expand Down Expand Up @@ -641,16 +633,6 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
// not blocked by any other passes' optimizations, so do it early.
P.addDifferentiabilityWitnessDevirtualizer();

if (!P.getOptions().EnableOSSAModules && SILDisableLateOMEByDefault) {
if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
return;

if (SILPrintFinalOSSAModule) {
addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
}
P.addNonTransparentFunctionOwnershipModelEliminator();
}

// Start by linking in referenced functions from other modules.
P.addPerformanceSILLinker();

Expand Down
2 changes: 2 additions & 0 deletions lib/SILOptimizer/Transforms/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,8 @@ bool SimplifyCFG::simplifyUnreachableBlock(UnreachableInst *UI) {
case SILInstructionKind::StrongReleaseInst:
case SILInstructionKind::RetainValueInst:
case SILInstructionKind::ReleaseValueInst:
case SILInstructionKind::DestroyValueInst:
case SILInstructionKind::EndBorrowInst:
break;
// We can only ignore a dealloc_stack instruction if we can ignore all
// instructions in the block.
Expand Down
24 changes: 17 additions & 7 deletions lib/SILOptimizer/Utils/InstructionDeleter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,28 @@ static bool isScopeAffectingInstructionDead(SILInstruction *inst,
return false;
}

// If inst has any owned move-only value as a result, deleting it may shorten
// that value's lifetime which is illegal according to language rules.
//
// In particular, this check is needed before returning true when
// getSingleValueCopyOrCast returns true. That function returns true for
// move_value instructions. And `move_value %moveOnlyValue` must not be
// deleted.
for (auto result : inst->getResults()) {
// If inst has any owned move-only value as a result, deleting it may
// shorten that value's lifetime which is illegal according to language
// rules.
//
// In particular, this check is needed before returning true when
// getSingleValueCopyOrCast returns true. That function returns true for
// move_value instructions. And `move_value %moveOnlyValue` must not be
// deleted.
if (result->getType().getASTType()->isNoncopyable() &&
result->getOwnershipKind() == OwnershipKind::Owned) {
return false;
}

// If result was lexical, lifetime shortening maybe observed, return.
if (result->isLexical()) {
auto resultTy = result->getType().getAs<SILFunctionType>();
// Allow deleted dead lexical values when they are trivial no escape types.
if (!resultTy || !resultTy->isTrivialNoEscape()) {
return false;
}
}
}

// If inst is a copy or beginning of scope, inst is dead, since we know that
Expand Down
5 changes: 5 additions & 0 deletions lib/SILOptimizer/Utils/OwnershipOptUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ bool OwnershipRAUWHelper::hasValidRAUWOwnership(SILValue oldValue,
if (m->getStage() == SILStage::Raw)
return false;

// OSSA rauw can create copies. Bail out if we have move only values.
if (newValue->getType().isMoveOnly()) {
return false;
}

return true;
}

Expand Down
1 change: 1 addition & 0 deletions test/Interpreter/moveonly_linkedlist.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all)
// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all)
// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all -Xfrontend -enable-ossa-modules)

// REQUIRES: executable_test

Expand Down
3 changes: 1 addition & 2 deletions test/SILOptimizer/consuming_parameter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ public func async_dead_arg_call(o: consuming AnyObject) async {

// CHECK-LABEL: sil [ossa] @async_dead_arg_call_lexical : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @noImplicitCopy @_lexical @owned
// CHECK: [[MOVE:%[^,]+]] = move_value [lexical] [[INSTANCE]]
// CHECK: [[EXECUTOR:%[^,]+]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
// CHECK: [[CALLEE:%[^,]+]] = function_ref @async_callee
// CHECK: apply [[CALLEE]]()
// CHECK: hop_to_executor [[EXECUTOR]]
// CHECK: destroy_value [[MOVE]]
// CHECK: destroy_value [[INSTANCE]]
// CHECK-LABEL: } // end sil function 'async_dead_arg_call_lexical'
@_silgen_name("async_dead_arg_call_lexical")
public func async_dead_arg_call_lexical(@_noEagerMove o: consuming AnyObject) async {
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/pre_specialize_layouts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public func usePrespecializedThrowsEntryPoints() throws {
// OPT-macosx: [[R9:%.*]] = unchecked_addr_cast [[R7]] : $*Array<Int> to $*Builtin.BridgeObject
// OPT-macosx: [[A3:%.*]] = unchecked_bitwise_cast [[P4]] : $Array<Int> to $Builtin.BridgeObject
// OPT-macosx: [[A4:%.*]] = unchecked_bitwise_cast [[P5]] : $Array<Float> to $Builtin.BridgeObject
// OPT-macosx: [[F2]]([[R8]], [[R9]], [[A3]], [[A4]], [[P3]]) : $@convention(thin) (@guaranteed Builtin.BridgeObject, @guaranteed Builtin.BridgeObject, Int64) -> (@out Builtin.BridgeObject, Int64, @out Builtin.BridgeObject) // user: %38
// OPT-macosx: [[F2]]([[R8]], [[R9]], [[A3]], [[A4]], [[P3]]) : $@convention(thin) (@guaranteed Builtin.BridgeObject, @guaranteed Builtin.BridgeObject, Int64) -> (@out Builtin.BridgeObject, Int64, @out Builtin.BridgeObject)
// OPT: } // end sil function '$s22pre_specialize_layouts40usePresepcializedMultipleIndirectResults___2xs2ysy0a20_specialized_module_C09SomeClassC_AA0m5OtherN0Cs5Int64VSaySiGSaySfGtF'
public final class SomeOtherClass {}
public func usePresepcializedMultipleIndirectResults(_ c: SomeClass, _ d: SomeOtherClass, _ x: Int64, xs: [Int], ys: [Float]) {
Expand Down
1 change: 0 additions & 1 deletion test/SILOptimizer/sil_combine_protocol_conf.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ public class Other {
// CHECK: [[W1:%.*]] = witness_method $@opened("{{.*}}", any DerivedProtocol) Self, #DerivedProtocol.foo : <Self where Self : DerivedProtocol> (Self) -> () -> Int, [[O1]] : $*@opened("{{.*}}", any DerivedProtocol) Self : $@convention(witness_method: DerivedProtocol) <τ_0_0 where τ_0_0 : DerivedProtocol> (@in_guaranteed τ_0_0) -> Int
// CHECK: apply [[W1]]<@opened("{{.*}}", any DerivedProtocol) Self>([[O1]]) : $@convention(witness_method: DerivedProtocol) <τ_0_0 where τ_0_0 : DerivedProtocol> (@in_guaranteed τ_0_0) -> Int
// CHECK: struct_extract
// CHECK: integer_literal
// CHECK: builtin
// CHECK: tuple_extract
// CHECK: tuple_extract
Expand Down
2 changes: 0 additions & 2 deletions test/stdlib/move_function.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ tests.test("simpleVarTest") {
expectTrue(_isUnique_native(&x))

var y = x
expectFalse(_isUnique_native(&x))
let _ = consume y
expectTrue(_isUnique_native(&x))
y = Klass()
Expand All @@ -101,7 +100,6 @@ tests.test("simpleVarTest") {
tests.test("simpleInoutVarTest") {
func inOutTest(_ x: inout Klass) {
var y = x
expectFalse(_isUnique_native(&x))
let _ = consume y
expectTrue(_isUnique_native(&x))
y = Klass()
Expand Down
6 changes: 3 additions & 3 deletions validation-test/SILOptimizer/lexical-lifetimes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class DataWrapper {
var pointer: UnsafeMutableRawBufferPointer

init(count: Int) {
pointer = UnsafeMutableRawBufferPointer.allocate(byteCount: count, alignment: MemoryLayout<Int>.alignment)
pointer = UnsafeMutableRawBufferPointer.allocate(byteCount: count * MemoryLayout<Int>.stride, alignment: MemoryLayout<Int>.alignment)
}

var bytes: UnsafeMutableRawBufferPointer { return pointer }
Expand All @@ -43,7 +43,7 @@ var fileHandleMap: [Int32 : String] = [:]

func openTheFile(_ path: String) -> Int32 {
let fd: Int32 = 42
assert(fileHandleMap[fd] == nil)
precondition(fileHandleMap[fd] == nil)
fileHandleMap[fd] = path
return fd
}
Expand All @@ -53,7 +53,7 @@ func closeTheFile(_ fd: Int32) {
}

func writeToTheFile(_ fd: Int32) {
assert(fileHandleMap[fd] != nil)
precondition(fileHandleMap[fd] != nil)
}

class FileHandleWrapper {
Expand Down