Skip to content

Commit

Permalink
This is part of a series of commits to remove reference forwarding fo…
Browse files Browse the repository at this point in the history
…r some of the ARC entry points. rdar://22724641.

After this commit, swift_retain will return no reference and LLVMARCContract pass is modified NOT to rewrite
swift_retain_noresult to old swift_retain which forwarded the reference.

Swift SVN r32075
  • Loading branch information
trentxintong committed Sep 18, 2015
1 parent 090e73c commit bc3fe16
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 223 deletions.
15 changes: 8 additions & 7 deletions include/swift/Runtime/HeapObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ extern "C" void swift_slowDealloc(void *ptr, size_t bytes, size_t alignMask);
/// Atomically increments the retain count of an object.
///
/// \param object - may be null, in which case this is a no-op
/// \return its argument value exactly
///
/// POSSIBILITIES: We may end up wanting a bunch of different variants:
/// - the general version which correctly handles null values, swift
Expand All @@ -147,16 +146,15 @@ extern "C" void swift_slowDealloc(void *ptr, size_t bytes, size_t alignMask);
/// - maybe a variant that can assume a non-null object
/// It may also prove worthwhile to have this use a custom CC
/// which preserves a larger set of registers.
extern "C" HeapObject *swift_retain(HeapObject *object);
extern "C" void swift_retain(HeapObject *object);
extern "C" void swift_retain_noresult(HeapObject *object);

extern "C" HeapObject *swift_retain_n(HeapObject *object, uint32_t n);
extern "C" void swift_retain_n(HeapObject *object, uint32_t n);

static inline HeapObject *_swift_retain_inlined(HeapObject *object) {
static inline void _swift_retain_inlined(HeapObject *object) {
if (object) {
object->refCount.increment();
}
return object;
}

/// Atomically increments the reference count of an object, unless it has
Expand Down Expand Up @@ -327,7 +325,9 @@ class SwiftRAII {
swift_release(object);
}

SwiftRAII(const SwiftRAII &other) : object(swift_retain(*other)) {
SwiftRAII(const SwiftRAII &other) {
swift_retain(*other);
object = *other;
;
}
SwiftRAII(SwiftRAII &&other) : object(*other) {
Expand All @@ -336,7 +336,8 @@ class SwiftRAII {
SwiftRAII &operator=(const SwiftRAII &other) {
if (object)
swift_release(object);
object = swift_retain(*other);
swift_retain(*other);
object = *other;
return *this;
}
SwiftRAII &operator=(SwiftRAII &&other) {
Expand Down
4 changes: 2 additions & 2 deletions include/swift/Runtime/InstrumentsSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ extern "C" HeapObject *(*_swift_allocObject)(HeapMetadata const *metadata,

extern "C" BoxPair::Return (*_swift_allocBox)(Metadata const *type);

extern "C" HeapObject *(*_swift_retain)(HeapObject *object);
extern "C" HeapObject *(*_swift_retain_n)(HeapObject *object, uint32_t n);
extern "C" void (*_swift_retain)(HeapObject *object);
extern "C" void (*_swift_retain_n)(HeapObject *object, uint32_t n);
extern "C" HeapObject *(*_swift_tryRetain)(HeapObject *object);
extern "C" bool (*_swift_isDeallocating)(HeapObject *object);
extern "C" void (*_swift_release)(HeapObject *object);
Expand Down
6 changes: 4 additions & 2 deletions lib/LLVMPasses/ARCEntryPointBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ class ARCEntryPointBuilder {
auto &M = getModule();
auto AttrList = AttributeSet::get(
M.getContext(), AttributeSet::FunctionIndex, Attribute::NoUnwind);
Retain = M.getOrInsertFunction("swift_retain", AttrList, ObjectPtrTy,
Retain = M.getOrInsertFunction("swift_retain", AttrList,
Type::getVoidTy(M.getContext()),
ObjectPtrTy, nullptr);
return Retain.get();
}
Expand Down Expand Up @@ -174,7 +175,8 @@ class ARCEntryPointBuilder {
auto *Int32Ty = Type::getInt32Ty(M.getContext());
auto AttrList = AttributeSet::get(
M.getContext(), AttributeSet::FunctionIndex, Attribute::NoUnwind);
RetainN = M.getOrInsertFunction("swift_retain_n", AttrList, ObjectPtrTy,
RetainN = M.getOrInsertFunction("swift_retain_n", AttrList,
Type::getVoidTy(M.getContext()),
ObjectPtrTy, Int32Ty, nullptr);
return RetainN.get();
}
Expand Down
145 changes: 6 additions & 139 deletions lib/LLVMPasses/LLVMARCContract.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ STATISTIC(NumRetainReleasesEliminatedByMergingIntoRetainReleaseN,
namespace {

struct LocalState {
Value *CurrentLocalUpdate;
TinyPtrVector<CallInst *> RetainList;
TinyPtrVector<CallInst *> ReleaseList;
};
Expand All @@ -46,9 +45,6 @@ struct LocalState {
///
/// Optimizations include:
///
/// - Lowering "retain no return" calls to swift_retain (which return the
/// retained argument) to lower register pressure.
///
/// - Merging together retain and release calls into retain_n, release_n
/// - calls.
///
Expand All @@ -63,31 +59,13 @@ class SwiftARCContractImpl {

/// The entry point builder that is used to construct ARC entry points.
ARCEntryPointBuilder B;

/// Since all of the calls are canonicalized, we know that we can just walk
/// through the function and collect the interesting heap object definitions
/// by getting the argument to these functions.
DenseMap<Value *, TinyPtrVector<Instruction *>> DefsOfValue;

/// Keep track of which order we see values in since iteration over a densemap
/// isn't in a deterministic order, and isn't efficient anyway.
///
/// TODO: Maybe this should be merged into DefsOfValue in a MapVector?
SmallVector<Value *, 16> DefOrder;

public:
SwiftARCContractImpl(Function &InF) : Changed(false), F(InF), B(F) {}

// The top level run routine of the pass.
bool run();

private:
/// Perform single basic block optimizations.
///
/// This means changing retain_no_return into retains, finding return values,
/// and merging retains, releases.
void performSingleBBOpts();

/// Perform the RRN Optimization given the current state that we are
/// tracking. This is called at the end of BBs and if we run into an unknown
/// call.
Expand All @@ -107,22 +85,11 @@ performRRNOptimization(DenseMap<Value *, LocalState> &PtrToLocalStateMap) {
if (RetainList.size() > 1) {
// Create the retainN call right by the first retain.
B.setInsertPoint(RetainList[0]);
auto &CI = *B.createRetainN(RetainList[0]->getArgOperand(0),
RetainList.size());

// Change the Local Entry to be the new retainN call.
P.second.CurrentLocalUpdate = &CI;

// Change GlobalEntry to track the new retainN instruction instead of
// the last retain that was seen.
TinyPtrVector<Instruction *> &GlobalEntry = DefsOfValue[ArgVal];
GlobalEntry.pop_back();
GlobalEntry.push_back(&CI);
B.createRetainN(RetainList[0]->getArgOperand(0), RetainList.size());

// Replace all uses of the retain instructions with our new retainN and
// then delete them.
for (auto *Inst : RetainList) {
Inst->replaceAllUsesWith(&CI);
Inst->eraseFromParent();
NumRetainReleasesEliminatedByMergingIntoRetainReleaseN++;
}
Expand Down Expand Up @@ -150,11 +117,9 @@ performRRNOptimization(DenseMap<Value *, LocalState> &PtrToLocalStateMap) {
}
}

void SwiftARCContractImpl::performSingleBBOpts() {
// Do a first pass over the function, collecting all interesting definitions.

// In this pass, we rewrite any intra-block uses that we can, since the
// SSAUpdater doesn't handle them.
bool SwiftARCContractImpl::run() {
// intra-BB retain/release merging.
DenseMap<Value *, LocalState> PtrToLocalStateMap;
for (BasicBlock &BB : F) {
for (auto II = BB.begin(), IE = BB.end(); II != IE; ) {
Expand All @@ -172,38 +137,11 @@ void SwiftARCContractImpl::performSingleBBOpts() {
case RT_Retain:
llvm_unreachable("This should be canonicalized away!");
case RT_RetainNoResult: {
auto *ArgVal = cast<CallInst>(Inst).getArgOperand(0);

B.setInsertPoint(&Inst);
CallInst &CI = *B.createRetain(ArgVal);
Inst.eraseFromParent();

if (!isa<Instruction>(ArgVal) && !isa<Argument>(ArgVal))
continue;

TinyPtrVector<Instruction *> &GlobalEntry = DefsOfValue[ArgVal];

// If this is the first definition of a value for the argument that
// we've seen, keep track of it in DefOrder.
if (GlobalEntry.empty())
DefOrder.push_back(ArgVal);
auto *CI = cast<CallInst>(&Inst);
auto *ArgVal = CI->getArgOperand(0);

LocalState &LocalEntry = PtrToLocalStateMap[ArgVal];

// Check to see if there is already an entry for this basic block. If
// there is another local entry, switch to using the local value and
// remove the previous value from the GlobalEntry.
if (LocalEntry.CurrentLocalUpdate) {
Changed = true;
CI.setArgOperand(0, LocalEntry.CurrentLocalUpdate);
assert(GlobalEntry.back() == LocalEntry.CurrentLocalUpdate &&
"Local/Global mismatch?");
GlobalEntry.pop_back();
}

LocalEntry.CurrentLocalUpdate = &CI;
LocalEntry.RetainList.push_back(&CI);
GlobalEntry.push_back(&CI);
LocalEntry.RetainList.push_back(CI);
continue;
}
case RT_Release: {
Expand All @@ -226,22 +164,9 @@ void SwiftARCContractImpl::performSingleBBOpts() {
case RT_CheckUnowned:
case RT_ObjCRelease:
case RT_ObjCRetain:
// Just remap any uses in the value.
break;
}

// Check to see if there are any uses of a value in the LocalUpdates
// map. If so, remap it now to the locally defined version.
for (unsigned i = 0, e = Inst.getNumOperands(); i != e; ++i) {
auto Iter = PtrToLocalStateMap.find(Inst.getOperand(i));
if (Iter != PtrToLocalStateMap.end()) {
if (Value *V = Iter->second.CurrentLocalUpdate) {
Changed = true;
Inst.setOperand(i, V);
}
}
}

if (Kind != RT_Unknown)
continue;

Expand Down Expand Up @@ -271,64 +196,6 @@ void SwiftARCContractImpl::performSingleBBOpts() {
performRRNOptimization(PtrToLocalStateMap);
PtrToLocalStateMap.clear();
}
}

bool SwiftARCContractImpl::run() {
// Perform single BB optimizations and gather information in prepration for
// the multiple BB optimizations.
performSingleBBOpts();

// Now that we've collected all of the interesting heap object values that are
// passed into argument-returning functions, rewrite uses of these pointers
// with optimized lifetime-shorted versions of it.
for (Value *Ptr : DefOrder) {
// If Ptr is an instruction, remember its block. If not, use the entry
// block as its block (it must be an argument, constant, etc).
BasicBlock *PtrBlock;
if (auto *PI = dyn_cast<Instruction>(Ptr))
PtrBlock = PI->getParent();
else
PtrBlock = &F.getEntryBlock();

TinyPtrVector<Instruction *> &Defs = DefsOfValue[Ptr];
// This is the same problem as SSA construction, so we just use LLVM's
// SSAUpdater, with each retain as a definition of the virtual value.
SSAUpdater Updater;
Updater.Initialize(Ptr->getType(), Ptr->getName());

// Set the return value of each of these calls as a definition of the
// virtual value.
for (auto *D : Defs)
Updater.AddAvailableValue(D->getParent(), D);

// If we didn't add a definition for Ptr's block, then Ptr itself is
// available in its block.
if (!Updater.HasValueForBlock(PtrBlock))
Updater.AddAvailableValue(PtrBlock, Ptr);

// Rewrite uses of Ptr to their optimized forms.
//
// NOTE: We are assuming that our Ptrs are not constants meaning that we
// know that users can not be constant expressions.
for (auto UI = Ptr->user_begin(), E = Ptr->user_end(); UI != E; ) {
// Make sure to increment the use iterator before potentially rewriting
// it.
Use &U = UI.getUse();
++UI;

// If the use is in the same block that defines it and the User is not a
// PHI node, then this is a local use that shouldn't be rewritten.
auto *User = cast<Instruction>(U.getUser());
if (User->getParent() == PtrBlock && !isa<PHINode>(User))
continue;

// Otherwise, change it if profitable!
Updater.RewriteUse(U);

if (U.get() != Ptr)
Changed = true;
}
}

return Changed;
}
Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/runtime/ErrorObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ swift::swift_getErrorValue(const SwiftError *errorObject,

SwiftError *
swift::swift_errorRetain(SwiftError *object) {
return static_cast<SwiftError*>(swift_retain(object));
swift_retain(object);
return static_cast<SwiftError*>(object);
}

void swift::swift_errorRelease(SwiftError *object) {
Expand Down
15 changes: 7 additions & 8 deletions stdlib/public/runtime/HeapObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,24 +281,23 @@ swift::swift_retain_noresult(HeapObject *object) {
swift_retain(object);
}

HeapObject *swift::swift_retain(HeapObject *object) {
void swift::swift_retain(HeapObject *object) {
SWIFT_RETAIN();
return _swift_retain(object);
_swift_retain(object);
}
static HeapObject *_swift_retain_(HeapObject *object) {
return _swift_retain_inlined(object);
static void _swift_retain_(HeapObject *object) {
_swift_retain_inlined(object);
}
auto swift::_swift_retain = _swift_retain_;

HeapObject *swift::swift_retain_n(HeapObject *object, uint32_t n) {
void swift::swift_retain_n(HeapObject *object, uint32_t n) {
SWIFT_RETAIN();
return _swift_retain_n(object, n);
_swift_retain_n(object, n);
}
static HeapObject *_swift_retain_n_(HeapObject *object, uint32_t n) {
static void _swift_retain_n_(HeapObject *object, uint32_t n) {
if (object) {
object->refCount.increment(n);
}
return object;
}
auto swift::_swift_retain_n = _swift_retain_n_;

Expand Down
6 changes: 4 additions & 2 deletions stdlib/public/runtime/MetadataImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ template <class Impl, class T> struct RetainableBoxBase {
struct SwiftRetainableBox :
RetainableBoxBase<SwiftRetainableBox, HeapObject*> {
static HeapObject *retain(HeapObject *obj) {
return swift_retain(obj);
swift_retain(obj);
return obj;
}

static void release(HeapObject *obj) {
Expand Down Expand Up @@ -443,7 +444,8 @@ struct UnknownRetainableBox : RetainableBoxBase<UnknownRetainableBox, void*> {
#if SWIFT_OBJC_INTEROP
return swift_unknownRetain(obj);
#else
return swift_retain(static_cast<HeapObject *>(obj));
swift_retain(static_cast<HeapObject *>(obj));
return static_cast<HeapObject *>(obj);
#endif
}

Expand Down
Loading

0 comments on commit bc3fe16

Please sign in to comment.