Skip to content

Remove the Array pinning addressor and all the pinning instructions in SIL #18922

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 4 commits into from
Aug 23, 2018
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: 1 addition & 1 deletion docs/ABI/Mangling.rst
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ Entities
ADDRESSOR-KIND ::= 'u' // unsafe addressor (no owner)
ADDRESSOR-KIND ::= 'O' // owning addressor (non-native owner)
ADDRESSOR-KIND ::= 'o' // owning addressor (native owner)
ADDRESSOR-KIND ::= 'p' // pinning addressor (native owner)
ADDRESSOR-KIND ::= 'p' // pinning addressor (native owner), not used anymore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since addressors never had public symbols do we still need to document and demangle them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need to. On the other hand it does not harm to be able to demangle Swift < 5 symbols, even if they are not public.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressors have public symbols when the storage implementation isn't resilient.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least you can remove mangling from the ASTMangler, even if you intend on keeping the demangler and remangler support.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did


decl-name ::= identifier
decl-name ::= identifier 'L' INDEX // locally-discriminated declaration
Expand Down
9 changes: 1 addition & 8 deletions docs/ARCOptimization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ Reference Counting Instructions
- fix_lifetime
- mark_dependence
- is_unique
- is_unique_or_pinned
- copy_block

Memory Behavior of ARC Operations
Expand Down Expand Up @@ -348,15 +347,10 @@ static argument type is optional, then a null check is also performed.
Thus, is_unique only returns true for non-null, native Swift object
references with a strong reference count of one.

is_unique_or_pinned has the same semantics as is_unique except that it
also returns true if the object is marked pinned (by strong_pin)
regardless of the reference count. This allows for simultaneous
non-structural modification of multiple subobjects.

Builtin.isUnique
----------------

Builtin.isUnique and Builtin.isUniqueOrPinned give the standard
Builtin.isUnique gives the standard
library access to optimization safe uniqueness checking. Because the
type of reference check is derived from the builtin argument's static
type, the most efficient check is automatically generated. However, in
Expand All @@ -367,7 +361,6 @@ additional pointer bit mask and dynamic class lookup to be bypassed in
these cases:

- isUnique_native : <T> (inout T[?]) -> Int1
- isUniqueOrPinned_native : <T> (inout T[?]) -> Int1

These builtins perform an implicit cast to NativeObject before
checking uniqueness. There's no way at SIL level to cast the address
Expand Down
26 changes: 0 additions & 26 deletions docs/SIL.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2881,16 +2881,6 @@ The second operand may have either object or address type. In the
latter case, the dependency is on the current value stored in the
address.

strong_pin
``````````

TODO: Fill me in!

strong_unpin
````````````

TODO: Fill me in!

is_unique
`````````

Expand All @@ -2909,22 +2899,6 @@ strong reference count is greater than 1.
A discussion of the semantics can be found here:
:ref:`arcopts.is_unique`.

is_unique_or_pinned
```````````````````

::

sil-instruction ::= 'is_unique_or_pinned' sil-operand

%1 = is_unique_or_pinned %0 : $*T
// $T must be a reference-counted type
// %1 will be of type Builtin.Int1

Checks whether %0 is the address of either a unique reference to a
memory object or a reference to a pinned object. Returns 1 if the
strong reference count is 1 or the object has been marked pinned by
strong_pin.

is_escaping_closure
```````````````````

Expand Down
2 changes: 0 additions & 2 deletions include/swift/AST/AccessorKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ ACCESSOR(Address)
IMMUTABLE_ADDRESSOR(Unsafe, unsafeAddress)
IMMUTABLE_ADDRESSOR(Owning, addressWithOwner)
IMMUTABLE_ADDRESSOR(NativeOwning, addressWithNativeOwner)
IMMUTABLE_ADDRESSOR(NativePinning, addressWithPinnedNativeOwner)

/// This is a mutableAddress-family accessor: a function that is
/// called when the storage is assigned to (like a setter) or
Expand All @@ -203,7 +202,6 @@ ACCESSOR(MutableAddress)
MUTABLE_ADDRESSOR(Unsafe, unsafeMutableAddress)
MUTABLE_ADDRESSOR(Owning, mutableAddressWithOwner)
MUTABLE_ADDRESSOR(NativeOwning, mutableAddressWithNativeOwner)
MUTABLE_ADDRESSOR(NativePinning, mutableAddressWithPinnedNativeOwner)

#ifdef LAST_ACCESSOR
LAST_ACCESSOR(MutableAddress)
Expand Down
17 changes: 0 additions & 17 deletions include/swift/AST/Builtins.def
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,6 @@ BUILTIN_SIL_OPERATION(Release, "release", Special)
/// autorelease: T -> ()
BUILTIN_SIL_OPERATION(Autorelease, "autorelease", Special)

// The pin operations return T only because Optional isn't intrinsic.

/// tryPin: Builtin.NativeObject -> T
BUILTIN_SIL_OPERATION(TryPin, "tryPin", Special)

/// unpin: T -> ()
BUILTIN_SIL_OPERATION(Unpin, "unpin", Special)

/// Load has type (Builtin.RawPointer) -> T
BUILTIN_SIL_OPERATION(Load, "load", Special)

Expand Down Expand Up @@ -359,21 +351,12 @@ BUILTIN_SIL_OPERATION(FixLifetime, "fixLifetime", Special)
/// references with a strong reference count of one.
BUILTIN_SIL_OPERATION(IsUnique, "isUnique", Special)

/// isUniqueOrPinned : <T> (inout T[?]) -> Int1
///
/// This builtin has the same semantics as isUnique except that it also returns
/// true if the object is marked pinned regardless of the reference count. This
/// allows for simultaneous non-structural modification of multiple subobjects.
BUILTIN_SIL_OPERATION(IsUniqueOrPinned, "isUniqueOrPinned", Special)

/// IsUnique_native : <T> (inout T[?]) -> Int1
///
/// These variants of isUnique implicitly cast to a non-null NativeObject before
/// checking uniqueness. This allows an object reference statically typed as
/// BridgeObject to be treated as a native object by the runtime.
BUILTIN_SIL_OPERATION(IsUnique_native, "isUnique_native", Special)
BUILTIN_SIL_OPERATION(IsUniqueOrPinned_native, "isUniqueOrPinned_native",
Special)

/// bindMemory : <T> (Builtin.RawPointer, Builtin.Word, T.Type) -> ()
BUILTIN_SIL_OPERATION(BindMemory, "bindMemory", Special)
Expand Down
3 changes: 0 additions & 3 deletions include/swift/AST/StorageImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ enum class AddressorKind : uint8_t {
/// \brief This is an owning addressor; it returns a Builtin.NativeObject
/// which should be released when the caller is done with the object.
NativeOwning,
/// \brief This is a pinning addressor; it returns a Builtin.NativeObject?
/// which should be unpinned when the caller is done with the object.
NativePinning,
};

/// Whether an access to storage is for reading, writing, or both.
Expand Down
16 changes: 0 additions & 16 deletions include/swift/SIL/SILBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1588,16 +1588,6 @@ class SILBuilder {
return insert(new (getModule()) StrongReleaseInst(
getSILDebugLocation(Loc), Operand, atomicity));
}
StrongPinInst *createStrongPin(SILLocation Loc, SILValue Operand,
Atomicity atomicity) {
return insert(new (getModule()) StrongPinInst(getSILDebugLocation(Loc),
Operand, atomicity));
}
StrongUnpinInst *createStrongUnpin(SILLocation Loc, SILValue Operand,
Atomicity atomicity) {
return insert(new (getModule()) StrongUnpinInst(getSILDebugLocation(Loc),
Operand, atomicity));
}

EndLifetimeInst *createEndLifetime(SILLocation Loc, SILValue Operand) {
return insert(new (getModule())
Expand Down Expand Up @@ -1632,12 +1622,6 @@ class SILBuilder {
return insert(new (getModule()) IsUniqueInst(getSILDebugLocation(Loc),
operand, Int1Ty));
}
IsUniqueOrPinnedInst *createIsUniqueOrPinned(SILLocation Loc,
SILValue value) {
auto Int1Ty = SILType::getBuiltinIntegerType(1, getASTContext());
return insert(new (getModule()) IsUniqueOrPinnedInst(
getSILDebugLocation(Loc), value, Int1Ty));
}
IsEscapingClosureInst *createIsEscapingClosure(SILLocation Loc,
SILValue operand,
unsigned VerificationType) {
Expand Down
29 changes: 0 additions & 29 deletions include/swift/SIL/SILCloner.h
Original file line number Diff line number Diff line change
Expand Up @@ -1940,26 +1940,6 @@ void SILCloner<ImplClass>::visitMarkDependenceInst(MarkDependenceInst *Inst) {
getOpValue(Inst->getBase())));
}

template<typename ImplClass>
void
SILCloner<ImplClass>::visitStrongPinInst(StrongPinInst *Inst) {
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
doPostProcess(Inst,
getBuilder().createStrongPin(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand()),
Inst->getAtomicity()));
}

template<typename ImplClass>
void
SILCloner<ImplClass>::visitStrongUnpinInst(StrongUnpinInst *Inst) {
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
doPostProcess(Inst,
getBuilder().createStrongUnpin(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand()),
Inst->getAtomicity()));
}

template<typename ImplClass>
void
SILCloner<ImplClass>::visitStrongReleaseInst(StrongReleaseInst *Inst) {
Expand All @@ -1977,15 +1957,6 @@ void SILCloner<ImplClass>::visitIsUniqueInst(IsUniqueInst *Inst) {
getBuilder().createIsUnique(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand())));
}
template<typename ImplClass>
void
SILCloner<ImplClass>::
visitIsUniqueOrPinnedInst(IsUniqueOrPinnedInst *Inst) {
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
doPostProcess(Inst,
getBuilder().createIsUniqueOrPinned(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand())));
}
template <typename ImplClass>
void SILCloner<ImplClass>::visitIsEscapingClosureInst(
IsEscapingClosureInst *Inst) {
Expand Down
67 changes: 0 additions & 67 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -4777,60 +4777,6 @@ class SetDeallocatingInst
}
};

/// StrongPinInst - Ensure that the operand is retained and pinned, if
/// not by this operation then by some enclosing pin.
///
/// Transformations must not do anything which reorders pin and unpin
/// operations. (This should generally be straightforward, as pin and
/// unpin may be conservatively assumed to have arbitrary
/// side-effects.)
///
/// This can't be a RefCountingInst because it returns a value.
class StrongPinInst
: public UnaryInstructionBase<SILInstructionKind::StrongPinInst,
SingleValueInstruction>
{
public:
using Atomicity = RefCountingInst::Atomicity;

private:
friend SILBuilder;

StrongPinInst(SILDebugLocation DebugLoc, SILValue operand,
Atomicity atomicity);

public:
void setAtomicity(Atomicity flag) {
SILInstruction::Bits.StrongPinInst.atomicity = bool(flag);
}
void setNonAtomic() {
SILInstruction::Bits.StrongPinInst.atomicity = bool(Atomicity::NonAtomic);
}
void setAtomic() {
SILInstruction::Bits.StrongPinInst.atomicity = bool(Atomicity::Atomic);
}
Atomicity getAtomicity() const {
return Atomicity(SILInstruction::Bits.StrongPinInst.atomicity);
}
bool isNonAtomic() const { return getAtomicity() == Atomicity::NonAtomic; }
bool isAtomic() const { return getAtomicity() == Atomicity::Atomic; }
};

/// StrongUnpinInst - Given that the operand is the result of a
/// strong_pin instruction, unpin it.
class StrongUnpinInst
: public UnaryInstructionBase<SILInstructionKind::StrongUnpinInst,
RefCountingInst>
{
friend SILBuilder;

StrongUnpinInst(SILDebugLocation DebugLoc, SILValue operand,
Atomicity atomicity)
: UnaryInstructionBase(DebugLoc, operand) {
setAtomicity(atomicity);
}
};

/// ObjectInst - Represents a object value type.
///
/// This instruction can only appear at the end of a gobal variable's
Expand Down Expand Up @@ -6261,19 +6207,6 @@ class IsUniqueInst
: UnaryInstructionBase(DebugLoc, Operand, BoolTy) {}
};

/// Given an object reference, return true iff it is non-nil and either refers
/// to a native swift object with strong reference count of 1 or refers to a
/// pinned object (for simultaneous access to multiple subobjects).
class IsUniqueOrPinnedInst
: public UnaryInstructionBase<SILInstructionKind::IsUniqueOrPinnedInst,
SingleValueInstruction> {
friend SILBuilder;

IsUniqueOrPinnedInst(SILDebugLocation DebugLoc, SILValue Operand,
SILType BoolTy)
: UnaryInstructionBase(DebugLoc, Operand, BoolTy) {}
};

/// Given an escaping closure return true iff it has a non-nil context and the
/// context has a strong reference count greater than 1.
class IsEscapingClosureInst
Expand Down
6 changes: 0 additions & 6 deletions include/swift/SIL/SILNodes.def
Original file line number Diff line number Diff line change
Expand Up @@ -369,17 +369,13 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
#include "swift/AST/ReferenceStorage.def"
SINGLE_VALUE_INST(UncheckedOwnershipConversionInst, unchecked_ownership_conversion,
SingleValueInstruction, MayHaveSideEffects, MayRelease)
SINGLE_VALUE_INST(StrongPinInst, strong_pin,
SingleValueInstruction, MayHaveSideEffects, DoesNotRelease)

// IsUnique does not actually write to memory but should be modeled
// as such. Its operand is a pointer to an object reference. The
// optimizer should not assume that the same object is pointed to after
// the isUnique instruction. It appears to write a new object reference.
SINGLE_VALUE_INST(IsUniqueInst, is_unique,
SingleValueInstruction, MayHaveSideEffects, DoesNotRelease)
SINGLE_VALUE_INST(IsUniqueOrPinnedInst, is_unique_or_pinned,
SingleValueInstruction, MayHaveSideEffects, DoesNotRelease)

SINGLE_VALUE_INST(IsEscapingClosureInst, is_escaping_closure,
SingleValueInstruction, MayRead, DoesNotRelease)
Expand Down Expand Up @@ -558,8 +554,6 @@ ABSTRACT_INST(RefCountingInst, SILInstruction)
RefCountingInst, MayHaveSideEffects, DoesNotRelease)
NON_VALUE_INST(StrongReleaseInst, strong_release,
RefCountingInst, MayHaveSideEffects, MayRelease)
NON_VALUE_INST(StrongUnpinInst, strong_unpin,
RefCountingInst, MayHaveSideEffects, DoesNotRelease)
NON_VALUE_INST(UnmanagedRetainValueInst, unmanaged_retain_value,
RefCountingInst, MayHaveSideEffects, DoesNotRelease)
NON_VALUE_INST(UnmanagedReleaseValueInst, unmanaged_release_value,
Expand Down
2 changes: 0 additions & 2 deletions include/swift/SILOptimizer/PassManager/Passes.def
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ PASS(PerformanceSILLinker, "performance-linker",
"Deserialize all referenced SIL functions")
PASS(RawSILInstLowering, "raw-sil-inst-lowering",
"Lower all raw SIL instructions to canonical equivalents.")
PASS(RemovePins, "remove-pins",
"Remove SIL pin/unpin pairs")
PASS(TempRValueOpt, "temp-rvalue-opt",
"Remove short-lived immutable temporary copies")
PASS(SideEffectsDumper, "side-effects-dump",
Expand Down
4 changes: 2 additions & 2 deletions include/swift/Serialization/ModuleFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t VERSION_MINOR = 436; // Last change: without_actually_escaping.
const uint16_t VERSION_MINOR = 437; // Last change: remove pinning

using DeclIDField = BCFixed<31>;

Expand Down Expand Up @@ -286,7 +286,7 @@ using MetatypeRepresentationField = BCFixed<2>;
// These IDs must \em not be renumbered or reordered without incrementing
// VERSION_MAJOR.
enum class AddressorKind : uint8_t {
NotAddressor, Unsafe, Owning, NativeOwning, NativePinning
NotAddressor, Unsafe, Owning, NativeOwning
};
using AddressorKindField = BCFixed<3>;

Expand Down
4 changes: 0 additions & 4 deletions lib/AST/ASTMangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ static StringRef getCodeForAccessorKind(AccessorKind kind,
return "lO";
case AddressorKind::NativeOwning:
return "lo";
case AddressorKind::NativePinning:
return "lp";
}
llvm_unreachable("bad addressor kind");
case AccessorKind::MutableAddress:
Expand All @@ -85,8 +83,6 @@ static StringRef getCodeForAccessorKind(AccessorKind kind,
return "aO";
case AddressorKind::NativeOwning:
return "ao";
case AddressorKind::NativePinning:
return "aP";
}
llvm_unreachable("bad addressor kind");
case AccessorKind::MaterializeForSet:
Expand Down
Loading