Skip to content

Follow-up changes for function effects #61641

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 15 commits into from
Oct 20, 2022
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
2 changes: 2 additions & 0 deletions SwiftCompilerSources/Sources/Basic/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ public struct StringRef : CustomStringConvertible, NoReflectionChildren {
}

public static func !=(lhs: StringRef, rhs: StaticString) -> Bool { !(lhs == rhs) }

public static func ~=(pattern: StaticString, value: StringRef) -> Bool { value == pattern }
}

//===----------------------------------------------------------------------===//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ public struct CalleeAnalysis {
// isDeinitBarrierFn:
{ (inst : BridgedInstruction, bca: BridgedCalleeAnalysis) -> Bool in
return inst.instruction.isDeinitBarrier(bca.analysis)
},
// getMemBehaviorFn
{ (bridgedCtxt: BridgedPassContext, bridgedApply: BridgedInstruction, observeRetains: Bool) -> BridgedMemoryBehavior in
let context = PassContext(_bridged: bridgedCtxt)
let apply = bridgedApply.instruction as! ApplySite
let e = context.calleeAnalysis.getSideEffects(of: apply)
return e.getMemBehavior(observeRetains: observeRetains)
}
)
}
Expand Down Expand Up @@ -52,6 +59,40 @@ public struct CalleeAnalysis {
}
return FunctionArray(bridged: bridgedDtors)
}

/// Returns the global (i.e. not argument specific) side effects of an apply.
public func getSideEffects(of apply: ApplySite) -> SideEffects.GlobalEffects {
guard let callees = getCallees(callee: apply.callee) else {
return .worstEffects
}

var result = SideEffects.GlobalEffects()
for callee in callees {
let calleeEffects = callee.getSideEffects()
result.merge(with: calleeEffects)
}
return result
}

/// Returns the argument specific side effects of an apply.
public func getSideEffects(of apply: ApplySite, forArgument argumentIdx: Int, path: SmallProjectionPath) -> SideEffects.GlobalEffects {
let calleeArgIdx = apply.calleeArgIndex(callerArgIndex: argumentIdx)
let convention = apply.getArgumentConvention(calleeArgIndex: calleeArgIdx)
let argument = apply.arguments[argumentIdx].at(path)

guard let callees = getCallees(callee: apply.callee) else {
return .worstEffects.restrictedTo(argument: argument, withConvention: convention)
}

var result = SideEffects.GlobalEffects()
for callee in callees {
let calleeEffects = callee.getSideEffects(forArgument: argument,
atIndex: calleeArgIdx,
withConvention: convention)
result.merge(with: calleeEffects)
}
return result.restrictedTo(argument: argument, withConvention: convention)
}
}

extension FullApplySite {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,14 @@ import SIL
let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
(function: Function, context: PassContext) in

var newEffects = Stack<EscapeEffects.ArgumentEffect>(context)
defer { newEffects.deinitialize() }
var newEffects = function.effects.escapeEffects.arguments.filter {!$0.isDerived }

let returnInst = function.returnInstruction
let argsWithDefinedEffects = getArgIndicesWithDefinedEscapingEffects(of: function)

for arg in function.arguments {
// We are not interested in arguments with trivial types.
if !arg.type.isNonTrivialOrContainsRawPointer(in: function) { continue }
if arg.hasTrivialNonPointerType { continue }

// Also, we don't want to override defined effects.
if argsWithDefinedEffects.contains(arg.index) { continue }
Expand All @@ -52,7 +51,7 @@ let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
context) {
let effect = EscapeEffects.ArgumentEffect(.notEscaping, argumentIndex: arg.index,
pathPattern: SmallProjectionPath(.anything))
newEffects.push(effect)
newEffects.append(effect)
continue
}

Expand All @@ -65,17 +64,22 @@ let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
}
}

// Don't modify the effects if they didn't change. This avoids sending a change notification
// which can trigger unnecessary other invalidations.
if newEffects == function.effects.escapeEffects.arguments {
return
}

context.modifyEffects(in: function) { (effects: inout FunctionEffects) in
effects.escapeEffects.arguments = effects.escapeEffects.arguments.filter { !$0.isDerived }
effects.escapeEffects.arguments.append(contentsOf: newEffects)
effects.escapeEffects.arguments = newEffects
}
})


/// Returns true if an argument effect was added.
private
func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
to newEffects: inout Stack<EscapeEffects.ArgumentEffect>,
to newEffects: inout [EscapeEffects.ArgumentEffect],
_ returnInst: ReturnInst?, _ context: PassContext) -> Bool {
// Correct the path if the argument is not a class reference itself, but a value type
// containing one or more references.
Expand Down Expand Up @@ -159,7 +163,7 @@ func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, exclusive),
argumentIndex: arg.index, pathPattern: argPath)
}
newEffects.push(effect)
newEffects.append(effect)
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ let computeSideEffects = FunctionPass(name: "compute-side-effects", {
collectedEffects.addEffectsForEcapingArgument(argument: argument)
}

// Don't modify the effects if they didn't change. This avoids sending a change notification
// which can trigger unnecessary other invalidations.
if let existingEffects = function.effects.sideEffects,
existingEffects.arguments == collectedEffects.argumentEffects,
existingEffects.global == collectedEffects.globalEffects {
return
}

// Finally replace the function's side effects.
context.modifyEffects(in: function) { (effects: inout FunctionEffects) in
effects.sideEffects = SideEffects(arguments: collectedEffects.argumentEffects, global: collectedEffects.globalEffects)
Expand Down Expand Up @@ -160,6 +168,11 @@ private struct CollectedEffects {
is CondFailInst:
break

case is BeginCOWMutationInst, is IsUniqueInst, is IsEscapingClosureInst:
// Model reference count reading as "destroy" for now. Although we could intoduce a "read-refcount"
// effect, it would not give any significant benefit in any of our current optimizations.
addEffects(.destroy, to: inst.operands[0].value, fromInitialPath: SmallProjectionPath(.anyValueFields))

default:
if inst.mayRelease {
globalEffects = .worstEffects
Expand Down Expand Up @@ -195,15 +208,17 @@ private struct CollectedEffects {

if escapeWalker.hasUnknownUses(argument: argument) {
// Worst case: we don't know anything about how the argument escapes.
addEffects(globalEffects.restrictedTo(argument: argument, withConvention: argument.convention), to: argument)
addEffects(globalEffects.restrictedTo(argument: argument.at(SmallProjectionPath(.anything)),
withConvention: argument.convention), to: argument)

} else if escapeWalker.foundTakingLoad {
// In most cases we can just ignore loads. But if the load is actually "taking" the
// underlying memory allocation, we must consider this as a "destroy", because we don't
// know what's happening with the loaded value. If there is any destroying instruction in the
// function, it might be the destroy of the loaded value.
let effects = SideEffects.GlobalEffects(ownership: globalEffects.ownership)
addEffects(effects.restrictedTo(argument: argument, withConvention: argument.convention), to: argument)
addEffects(effects.restrictedTo(argument: argument.at(SmallProjectionPath(.anything)),
withConvention: argument.convention), to: argument)

} else if escapeWalker.foundConsumingPartialApply && globalEffects.ownership.destroy {
// Similar situation with apply instructions which consume the callee closure.
Expand All @@ -228,7 +243,7 @@ private struct CollectedEffects {
} else {
// The callee doesn't have any computed effects. At least we can do better
// if it has any defined effect attribute (like e.g. `[readnone]`).
globalEffects.merge(with: callee.definedSideEffects)
globalEffects.merge(with: callee.definedGlobalEffects)
}
}

Expand All @@ -244,15 +259,12 @@ private struct CollectedEffects {
if let calleePath = calleeEffect.copy { addEffects(.copy, to: argument, fromInitialPath: calleePath) }
if let calleePath = calleeEffect.destroy { addEffects(.destroy, to: argument, fromInitialPath: calleePath) }
} else {
var calleeEffects = callee.definedSideEffects

let convention = callee.getArgumentConvention(for: calleeArgIdx)
if convention.isIndirectIn {
calleeEffects.memory.read = true
} else if convention == .indirectOut {
calleeEffects.memory.write = true
}
addEffects(calleeEffects.restrictedTo(argument: argument, withConvention: convention), to: argument)
let wholeArgument = argument.at(SmallProjectionPath(.anything))
let calleeEffects = callee.getSideEffects(forArgument: wholeArgument,
atIndex: calleeArgIdx,
withConvention: convention)
addEffects(calleeEffects.restrictedTo(argument: wholeArgument, withConvention: convention), to: argument)
}
}
}
Expand All @@ -266,7 +278,7 @@ private struct CollectedEffects {
// deallocates an object.
if let destructors = calleeAnalysis.getDestructors(of: addressOrValue.type) {
for destructor in destructors {
globalEffects.merge(with: destructor.effects.accumulatedSideEffects)
globalEffects.merge(with: destructor.getSideEffects())
}
} else {
globalEffects = .worstEffects
Expand Down Expand Up @@ -368,7 +380,7 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
mutating func hasUnknownUses(argument: FunctionArgument) -> Bool {
if argument.type.isAddress {
return walkDownUses(ofAddress: argument, path: UnusedWalkingPath()) == .abortWalk
} else if argument.type.isTrivial(in: argument.function) {
} else if argument.hasTrivialType {
return false
} else {
return walkDownUses(ofValue: argument, path: UnusedWalkingPath()) == .abortWalk
Expand Down Expand Up @@ -407,22 +419,22 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
switch inst {
case let copy as CopyAddrInst:
if address == copy.sourceOperand &&
!address.value.type.isTrivial(in: inst.function) &&
!address.value.hasTrivialType &&
(!function.hasOwnership || copy.isTakeOfSrc) {
foundTakingLoad = true
}
return .continueWalk

case let load as LoadInst:
if !address.value.type.isTrivial(in: function) &&
if !address.value.hasTrivialType &&
// In non-ossa SIL we don't know if a load is taking.
(!function.hasOwnership || load.ownership == .take) {
foundTakingLoad = true
}
return .continueWalk

case is LoadWeakInst, is LoadUnownedInst, is LoadBorrowInst:
if !function.hasOwnership && !address.value.type.isTrivial(in: function) {
if !function.hasOwnership && !address.value.hasTrivialType {
foundTakingLoad = true
}
return .continueWalk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ private struct FindAllocationOfRelease : ValueUseDefWalker {
//
var nonTrivialOperandFound = false
for operand in def.operands {
if !operand.value.type.isTrivial(in: def.function) {
if !operand.value.hasTrivialType {
if nonTrivialOperandFound {
return .abortWalk
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct PassContext {
}

func modifyEffects(in function: Function, _ body: (inout FunctionEffects) -> ()) {
notifyFunctionDataChanged()
notifyEffectsChanged()
function._modifyEffects(body)
}

Expand All @@ -139,8 +139,8 @@ struct PassContext {
PassContext_notifyChanges(_bridged, branchesChanged)
}

fileprivate func notifyFunctionDataChanged() {
PassContext_notifyChanges(_bridged, functionDataChanged)
fileprivate func notifyEffectsChanged() {
PassContext_notifyChanges(_bridged, effectsChanged)
}
}

Expand Down Expand Up @@ -243,7 +243,7 @@ extension RefCountingInst {

extension Function {
func set(needStackProtection: Bool, _ context: PassContext) {
context.notifyFunctionDataChanged()
context.notifyEffectsChanged()
SILFunction_setNeedStackProtection(bridged, needStackProtection ? 1 : 0)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,11 @@ extension EscapeVisitor {
}

func hasRelevantType(_ value: Value, at path: SmallProjectionPath, analyzeAddresses: Bool) -> Bool {
let type = value.type
if type.isNonTrivialOrContainsRawPointer(in: value.function) { return true }
if !value.hasTrivialNonPointerType { return true }

// For selected addresses we also need to consider trivial types (`value`
// is a selected address if the path does not contain any class projections).
if analyzeAddresses && type.isAddress && !path.hasClassProjection { return true }
if analyzeAddresses && value.type.isAddress && !path.hasClassProjection { return true }
return false
}
}
Expand Down Expand Up @@ -542,14 +541,15 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
let svi = instruction as! SingleValueInstruction

// Even when analyzing addresses, a loaded trivial value can be ignored.
if !svi.type.isNonTrivialOrContainsRawPointer(in: svi.function) { return .continueWalk }
if svi.hasTrivialNonPointerType { return .continueWalk }
return walkDownUses(ofValue: svi, path: path.with(knownType: nil))
case let atp as AddressToPointerInst:
return walkDownUses(ofValue: atp, path: path.with(knownType: nil))
case let ia as IndexAddrInst:
assert(operand.index == 0)
return walkDownUses(ofAddress: ia, path: path.with(knownType: nil))
case is DeallocStackInst, is InjectEnumAddrInst, is FixLifetimeInst, is EndBorrowInst, is EndAccessInst:
case is DeallocStackInst, is InjectEnumAddrInst, is FixLifetimeInst, is EndBorrowInst, is EndAccessInst,
is DebugValueInst:
return .continueWalk
default:
return isEscaping
Expand Down
7 changes: 7 additions & 0 deletions SwiftCompilerSources/Sources/SIL/ApplySite.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,11 @@ extension FullApplySite {
}
return nil
}

/// The number of indirect out arguments.
///
/// 0 if the callee has a direct or no return value and 1, if it has an indirect return value.
public var numIndirectResultArguments: Int {
return FullApplySite_numIndirectResultArguments(bridged)
}
}
Loading