Skip to content

SILGen: Fix order of operations when a mutating existential method returns Self. #18937

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
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
3 changes: 3 additions & 0 deletions include/swift/SIL/SILType.h
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,9 @@ class SILType {
/// Returns the underlying referent SILType of an @sil_unowned or @sil_weak
/// Type.
SILType getReferentType(SILModule &M) const;

/// Returns a SILType with any archetypes mapped out of context.
SILType mapTypeOutOfContext() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind splitting this off into a separate commit and doing a quick pass over mapTypeOutOfContext() calls throughout SIL to see if it can be used anywhere else?


/// Given two SIL types which are representations of the same type,
/// check whether they have an abstraction difference.
Expand Down
5 changes: 1 addition & 4 deletions lib/IRGen/GenHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1488,10 +1488,7 @@ class FixedBoxTypeInfoBase : public BoxTypeInfo {
// Allocate a new object using the layout.
auto boxedInterfaceType = boxedType;
if (env) {
boxedInterfaceType = SILType::getPrimitiveType(
boxedType.getASTType()->mapTypeOutOfContext()
->getCanonicalType(),
boxedType.getCategory());
boxedInterfaceType = boxedType.mapTypeOutOfContext();
}

auto boxDescriptor = IGF.IGM.getAddrOfBoxDescriptor(
Expand Down
6 changes: 6 additions & 0 deletions lib/SIL/SILType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ SILType SILType::getReferentType(SILModule &M) const {
return M.Types.getLoweredType(Ty->getReferentType()->getCanonicalType());
}

SILType SILType::mapTypeOutOfContext() const {
return SILType::getPrimitiveType(getASTType()->mapTypeOutOfContext()
->getCanonicalType(),
getCategory());
}

CanType
SILBoxType::getFieldLoweredType(SILModule &M, unsigned index) const {
auto fieldTy = getLayout()->getFields()[index].getLoweredType();
Expand Down
4 changes: 4 additions & 0 deletions lib/SILGen/Cleanup.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ class LLVM_LIBRARY_VISIBILITY CleanupManager {
assert(!stack.empty());
return stack.stable_begin();
}

Cleanup &getCleanup(CleanupHandle iter) {
return *stack.find(iter);
}

/// \brief Emit a branch to the given jump destination,
/// threading out through any cleanups we need to run. This does not pop the
Expand Down
170 changes: 163 additions & 7 deletions lib/SILGen/ResultPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "LValue.h"
#include "RValue.h"
#include "SILGenFunction.h"
#include "swift/AST/GenericEnvironment.h"

using namespace swift;
using namespace Lowering;
Expand All @@ -29,7 +30,7 @@ namespace {

/// A result plan for evaluating an indirect result into the address
/// associated with an initialization.
class InPlaceInitializationResultPlan : public ResultPlan {
class InPlaceInitializationResultPlan final : public ResultPlan {
Initialization *init;

public:
Expand All @@ -47,10 +48,155 @@ class InPlaceInitializationResultPlan : public ResultPlan {
}
};

/// A cleanup that handles the delayed emission of an indirect buffer for opened
/// Self arguments.
class IndirectOpenedSelfCleanup final : public Cleanup {
SILValue box;
public:
IndirectOpenedSelfCleanup()
: box()
{}

void setBox(SILValue b) {
assert(!box && "buffer already set?!");
box = b;
}

void emit(SILGenFunction &SGF, CleanupLocation loc, ForUnwind_t forUnwind)
override {
assert(box && "buffer never emitted before activating cleanup?!");
SGF.B.createDeallocBox(loc, box);
}

void dump(SILGenFunction &SGF) const override {
llvm::errs() << "IndirectOpenedSelfCleanup\n";
if (box)
box->dump();
}
};

/// Map a type expressed in terms of opened archetypes into a context-free
/// dependent type, returning the type, a generic signature with parameters
/// corresponding to each opened type,
static std::tuple<CanType, CanGenericSignature, SubstitutionMap>
mapTypeOutOfOpenedExistentialContext(CanType t) {
SmallVector<ArchetypeType *, 4> openedTypes;
t->getOpenedExistentials(openedTypes);

ArrayRef<Type> openedTypesAsTypes(
reinterpret_cast<const Type *>(openedTypes.data()),
openedTypes.size());

SmallVector<GenericTypeParamType *, 4> params;
for (unsigned i : indices(openedTypes)) {
params.push_back(GenericTypeParamType::get(0, i, t->getASTContext()));
}

auto mappedSig = GenericSignature::get(params, {});
auto mappedSubs = SubstitutionMap::get(mappedSig, openedTypesAsTypes, {});

auto mappedTy = t.subst(
[&](SubstitutableType *t) -> Type {
auto index = std::find(openedTypes.begin(), openedTypes.end(), t)
- openedTypes.begin();
assert(index != openedTypes.end() - openedTypes.begin());
return params[index];
},
MakeAbstractConformanceForGenericType());

return std::make_tuple(mappedTy->getCanonicalType(mappedSig),
mappedSig->getCanonicalSignature(),
mappedSubs);
}

/// A result plan for an indirectly-returned opened existential value.
///
/// This defers allocating the temporary for the result to a later point so that
/// it happens after the arguments are evaluated.
class IndirectOpenedSelfResultPlan final : public ResultPlan {
AbstractionPattern origType;
CanType substType;
CleanupHandle handle = CleanupHandle::invalid();
mutable SILValue resultBox, resultBuf;

public:
IndirectOpenedSelfResultPlan(SILGenFunction &SGF,
AbstractionPattern origType,
CanType substType)
: origType(origType), substType(substType)
{
// Create a cleanup to deallocate the stack buffer at the proper scope.
// We won't emit the buffer till later, after arguments have been opened,
// though.
SGF.Cleanups.pushCleanupInState<IndirectOpenedSelfCleanup>(
CleanupState::Dormant);
handle = SGF.Cleanups.getCleanupsDepth();
}

void
gatherIndirectResultAddrs(SILGenFunction &SGF, SILLocation loc,
SmallVectorImpl<SILValue> &outList) const override {
assert(!resultBox && "already created temporary?!");

// We allocate the buffer as a box because the scope nesting won't clean
// this up with good stack discipline relative to any stack allocations that
// occur during argument emission. Escape analysis during mandatory passes
// ought to clean this up.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add a test for that. However another approach would be to allocate a stack buffer of existential type and then open the existential late, using the same opened type as the result of the call. Would that make sense? Then at least you avoid the heap allocation if the value fits inline.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a case where we're always going to erase the existential after the call, right? Can we shift that erasure into the result plan so that we emit into the address produced by init_existential_addr? That would also let us actually do initialization into the context if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that’s basically what I’m suggesting.


auto resultTy = SGF.getLoweredType(origType, substType).getASTType();
CanType layoutTy;
CanGenericSignature layoutSig;
SubstitutionMap layoutSubs;
std::tie(layoutTy, layoutSig, layoutSubs)
= mapTypeOutOfOpenedExistentialContext(resultTy);

auto boxLayout = SILLayout::get(SGF.getASTContext(),
layoutSig->getCanonicalSignature(),
SILField(layoutTy->getCanonicalType(layoutSig), true));

resultBox = SGF.B.createAllocBox(loc,
SILBoxType::get(SGF.getASTContext(),
boxLayout,
layoutSubs));

// Complete the cleanup to deallocate this buffer later, after we're
// finished with the argument.
static_cast<IndirectOpenedSelfCleanup&>(SGF.Cleanups.getCleanup(handle))
.setBox(resultBox);
SGF.Cleanups.setCleanupState(handle, CleanupState::Active);

resultBuf = SGF.B.createProjectBox(loc, resultBox, 0);
outList.emplace_back(resultBuf);
}

RValue finish(SILGenFunction &SGF, SILLocation loc, CanType substType,
ArrayRef<ManagedValue> &directResults) override {
assert(resultBox && "never emitted temporary?!");

// Lower the unabstracted result type.
auto &substTL = SGF.getTypeLowering(substType);

ManagedValue value;
// If the value isn't address-only, go ahead and load.
if (!substTL.isAddressOnly()) {
auto load = substTL.emitLoad(SGF.B, loc, resultBuf,
LoadOwnershipQualifier::Take);
value = SGF.emitManagedRValueWithCleanup(load);
} else {
value = SGF.emitManagedRValueWithCleanup(resultBuf);
}

// A Self return should never be further abstracted. It's also never emitted
// into context; we disable that optimization because Self may not even
// be available to pre-allocate a stack buffer before we prepare a call.
return RValue(SGF, loc, substType, value);
}
};

/// A result plan for working with a single value and potentially
/// reabstracting it. The value can actually be a tuple if the
/// abstraction is opaque.
class ScalarResultPlan : public ResultPlan {
class ScalarResultPlan final : public ResultPlan {
std::unique_ptr<TemporaryInitialization> temporary;
AbstractionPattern origType;
Initialization *init;
Expand Down Expand Up @@ -150,7 +296,7 @@ class ScalarResultPlan : public ResultPlan {

/// A result plan which calls copyOrInitValueInto on an Initialization
/// using a temporary buffer initialized by a sub-plan.
class InitValueFromTemporaryResultPlan : public ResultPlan {
class InitValueFromTemporaryResultPlan final : public ResultPlan {
Initialization *init;
ResultPlanPtr subPlan;
std::unique_ptr<TemporaryInitialization> temporary;
Expand Down Expand Up @@ -184,7 +330,7 @@ class InitValueFromTemporaryResultPlan : public ResultPlan {

/// A result plan which calls copyOrInitValueInto using the result of
/// a sub-plan.
class InitValueFromRValueResultPlan : public ResultPlan {
class InitValueFromRValueResultPlan final : public ResultPlan {
Initialization *init;
ResultPlanPtr subPlan;

Expand Down Expand Up @@ -212,7 +358,7 @@ class InitValueFromRValueResultPlan : public ResultPlan {

/// A result plan which produces a larger RValue from a bunch of
/// components.
class TupleRValueResultPlan : public ResultPlan {
class TupleRValueResultPlan final : public ResultPlan {
SmallVector<ResultPlanPtr, 4> eltPlans;

public:
Expand Down Expand Up @@ -254,7 +400,7 @@ class TupleRValueResultPlan : public ResultPlan {

/// A result plan which evaluates into the sub-components
/// of a splittable tuple initialization.
class TupleInitializationResultPlan : public ResultPlan {
class TupleInitializationResultPlan final : public ResultPlan {
Initialization *tupleInit;
SmallVector<InitializationPtr, 4> eltInitsBuffer;
MutableArrayRef<InitializationPtr> eltInits;
Expand Down Expand Up @@ -305,7 +451,7 @@ class TupleInitializationResultPlan : public ResultPlan {
}
};

class ForeignErrorInitializationPlan : public ResultPlan {
class ForeignErrorInitializationPlan final : public ResultPlan {
SILLocation loc;
LValue lvalue;
ResultPlanPtr subPlan;
Expand Down Expand Up @@ -466,6 +612,16 @@ ResultPlanPtr ResultPlanBuilder::build(Initialization *init,
// - store it to the destination
// We could break this down into different ResultPlan implementations,
// but it's easier not to.

// If the result type involves an indirectly-returned opened existential,
// then we need to evaluate the arguments first in order to have access to
// the opened Self type. A special result plan defers allocating the stack
// slot to the point the call is emitted.
if (result.getType()->hasOpenedExistential()
&& SGF.silConv.isSILIndirect(result)) {
return ResultPlanPtr(
new IndirectOpenedSelfResultPlan(SGF, origType, substType));
}

// Create a temporary if the result is indirect.
std::unique_ptr<TemporaryInitialization> temporary;
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4516,7 +4516,7 @@ RValue SILGenFunction::emitApply(ResultPlanPtr &&resultPlan,

auto directResultsArray = makeArrayRef(directResults);
RValue result =
resultPlan->finish(*this, loc, substResultType, directResultsArray);
resultPlan->finish(*this, loc, substResultType, directResultsArray);
assert(directResultsArray.empty() && "didn't claim all direct results");

return result;
Expand Down
Loading