Skip to content

Commit 030b21e

Browse files
swift-ciswift-ci
authored
Merge pull request #18891 from rjmccall/inout-reabstraction
2 parents f2a2376 + c491f1b commit 030b21e

File tree

7 files changed

+167
-40
lines changed

7 files changed

+167
-40
lines changed

‎include/swift/Basic/DiverseStack.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,14 @@ template <class T> class DiverseStackImpl : private DiverseStackBase {
366366
void pop(stable_iterator stable_iter) {
367367
iterator iter = find(stable_iter);
368368
checkIterator(iter);
369+
#ifndef NDEBUG
369370
while (Begin != iter.Ptr) {
370371
pop();
371372
checkIterator(iter);
372373
}
374+
#else
375+
Begin = iter.Ptr;
376+
#endif
373377
}
374378
};
375379

‎lib/SILGen/Cleanup.cpp

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -75,45 +75,52 @@ void CleanupManager::popAndEmitCleanup(CleanupHandle handle,
7575

7676
void CleanupManager::emitCleanups(CleanupsDepth depth, CleanupLocation loc,
7777
ForUnwind_t forUnwind, bool popCleanups) {
78-
auto begin = stack.stable_begin();
79-
while (begin != depth) {
80-
auto iter = stack.find(begin);
81-
78+
auto cur = stack.stable_begin();
79+
#ifndef NDEBUG
80+
auto topOfStack = cur;
81+
#endif
82+
while (cur != depth) {
83+
// Copy the cleanup off the stack if it needs to be emitted.
84+
// This is necessary both because we might need to pop the cleanup and
85+
// because the cleanup might push other cleanups that will invalidate
86+
// references onto the stack.
87+
auto iter = stack.find(cur);
8288
Cleanup &stackCleanup = *iter;
89+
Optional<CleanupBuffer> copiedCleanup;
90+
if (stackCleanup.isActive() && SGF.B.hasValidInsertionPoint()) {
91+
copiedCleanup.emplace(stackCleanup);
92+
}
8393

84-
// Copy it off the cleanup stack in case the cleanup pushes a new cleanup
85-
// and the backing storage is re-allocated.
86-
CleanupBuffer buffer(stackCleanup);
87-
Cleanup &cleanup = buffer.getCopy();
88-
89-
// Advance stable iterator.
90-
begin = stack.stabilize(++iter);
94+
// Advance the iterator.
95+
cur = stack.stabilize(++iter);
9196

92-
// Pop now.
93-
if (popCleanups)
97+
// Pop now if that was requested.
98+
if (popCleanups) {
9499
stack.pop();
95100

96-
if (cleanup.isActive() && SGF.B.hasValidInsertionPoint())
97-
cleanup.emit(SGF, loc, forUnwind);
101+
#ifndef NDEBUG
102+
topOfStack = stack.stable_begin();
103+
#endif
104+
}
98105

99-
stack.checkIterator(begin);
106+
// Emit the cleanup.
107+
if (copiedCleanup) {
108+
copiedCleanup->getCopy().emit(SGF, loc, forUnwind);
109+
#ifndef NDEBUG
110+
if (hasAnyActiveCleanups(stack.stable_begin(), topOfStack)) {
111+
copiedCleanup->getCopy().dump(SGF);
112+
llvm_unreachable("cleanup left active cleanups on stack");
113+
}
114+
#endif
115+
}
116+
117+
stack.checkIterator(cur);
100118
}
101119
}
102120

103-
/// Leave a scope, with all its cleanups.
121+
/// Leave a scope, emitting all the cleanups that are currently active.
104122
void CleanupManager::endScope(CleanupsDepth depth, CleanupLocation loc) {
105-
stack.checkIterator(depth);
106-
107-
// FIXME: Thread a branch through the cleanups if there are any active
108-
// cleanups and we have a valid insertion point.
109-
110-
if (!::hasAnyActiveCleanups(stack.begin(), stack.find(depth))) {
111-
return;
112-
}
113-
114-
// Iteratively mark cleanups dead and pop them.
115-
// Maybe we'd get better results if we marked them all dead in one shot?
116-
emitCleanups(depth, loc, NotForUnwind);
123+
emitCleanups(depth, loc, NotForUnwind, /*popCleanups*/ true);
117124
}
118125

119126
bool CleanupManager::hasAnyActiveCleanups(CleanupsDepth from,

‎lib/SILGen/Cleanup.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,7 @@ class LLVM_LIBRARY_VISIBILITY CleanupManager {
136136

137137
void popTopDeadCleanups(CleanupsDepth end);
138138
void emitCleanups(CleanupsDepth depth, CleanupLocation l,
139-
ForUnwind_t forUnwind,
140-
bool popCleanups=true);
139+
ForUnwind_t forUnwind, bool popCleanups);
141140
void endScope(CleanupsDepth depth, CleanupLocation l);
142141

143142
Cleanup &initCleanup(Cleanup &cleanup, size_t allocSize, CleanupState state);

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ struct LValueWritebackCleanup : Cleanup {
4747

4848
void emit(SILGenFunction &SGF, CleanupLocation loc,
4949
ForUnwind_t forUnwind) override {
50+
FullExpr scope(SGF.Cleanups, loc);
51+
5052
// TODO: honor forUnwind!
5153
auto &evaluation = *SGF.FormalEvalContext.find(Depth);
5254
assert(evaluation.getKind() == FormalAccess::Exclusive);

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,53 @@ static CanType getSingleTupleElement(CanType type) {
827827
}
828828

829829
namespace {
830+
class TranslateIndirect : public Cleanup {
831+
AbstractionPattern InputOrigType, OutputOrigType;
832+
CanType InputSubstType, OutputSubstType;
833+
SILValue Input, Output;
834+
835+
public:
836+
TranslateIndirect(AbstractionPattern inputOrigType, CanType inputSubstType,
837+
AbstractionPattern outputOrigType, CanType outputSubstType,
838+
SILValue input, SILValue output)
839+
: InputOrigType(inputOrigType), OutputOrigType(outputOrigType),
840+
InputSubstType(inputSubstType), OutputSubstType(outputSubstType),
841+
Input(input), Output(output) {
842+
assert(input->getType().isAddress());
843+
assert(output->getType().isAddress());
844+
}
845+
846+
void emit(SILGenFunction &SGF, CleanupLocation loc,
847+
ForUnwind_t forUnwind) override {
848+
FullExpr scope(SGF.Cleanups, loc);
849+
850+
// Re-assert ownership of the input value.
851+
auto inputMV = SGF.emitManagedBufferWithCleanup(Input);
852+
853+
// Set up an initialization of the output buffer.
854+
auto &outputTL = SGF.getTypeLowering(Output->getType());
855+
auto outputInit = SGF.useBufferAsTemporary(Output, outputTL);
856+
857+
// Transform into the output buffer.
858+
auto mv = SGF.emitTransformedValue(loc, inputMV,
859+
InputOrigType, InputSubstType,
860+
OutputOrigType, OutputSubstType,
861+
SGFContext(outputInit.get()));
862+
emitForceInto(SGF, loc, mv, *outputInit);
863+
864+
// Disable the cleanup; we've kept our promise to leave the inout
865+
// initialized.
866+
outputInit->getManagedAddress().forward(SGF);
867+
}
868+
869+
void dump(SILGenFunction &SGF) const override {
870+
llvm::errs() << "TranslateIndirect("
871+
<< InputOrigType << ", " << InputSubstType << ", "
872+
<< OutputOrigType << ", " << OutputSubstType << ", "
873+
<< Output << ", " << Input << ")\n";
874+
}
875+
};
876+
830877
class TranslateArguments {
831878
SILGenFunction &SGF;
832879
SILLocation Loc;
@@ -1356,13 +1403,49 @@ namespace {
13561403
outputSubstType, input);
13571404
return;
13581405
case ParameterConvention::Indirect_Inout: {
1359-
// If it's inout, we need writeback.
1360-
llvm::errs() << "inout writeback in abstraction difference thunk "
1361-
"not yet implemented\n";
1362-
llvm::errs() << "input value ";
1363-
input.getValue()->dump();
1364-
llvm::errs() << "output type " << SGF.getSILType(result) << "\n";
1365-
abort();
1406+
inputOrigType = inputOrigType.getWithoutSpecifierType();
1407+
inputSubstType = inputSubstType.getWithoutSpecifierType();
1408+
outputOrigType = outputOrigType.getWithoutSpecifierType();
1409+
outputSubstType = outputSubstType.getWithoutSpecifierType();
1410+
1411+
// Create a temporary of the right type.
1412+
auto &temporaryTL = SGF.getTypeLowering(result.getType());
1413+
auto temporary = SGF.emitTemporary(Loc, temporaryTL);
1414+
1415+
// Take ownership of the input value. This leaves the input l-value
1416+
// effectively uninitialized, but we'll push a cleanup that will put
1417+
// a value back into it.
1418+
FullExpr scope(SGF.Cleanups, CleanupLocation::get(Loc));
1419+
auto ownedInput =
1420+
SGF.emitManagedBufferWithCleanup(input.getLValueAddress());
1421+
1422+
// Translate the input value into the temporary.
1423+
translateSingleInto(inputOrigType, inputSubstType,
1424+
outputOrigType, outputSubstType,
1425+
ownedInput, *temporary);
1426+
1427+
// Forward the cleanup on the temporary. We're about to push a new
1428+
// cleanup that will re-assert ownership of this value.
1429+
auto temporaryAddr = temporary->getManagedAddress().forward(SGF);
1430+
1431+
// Leave the scope in which we did the forward translation. This
1432+
// ensures that the value in the input buffer is destroyed
1433+
// immediately rather than (potentially) arbitrarily later
1434+
// at a point where we want to put new values in the input buffer.
1435+
scope.pop();
1436+
1437+
// Push the cleanup to perform the reverse translation. This cleanup
1438+
// asserts ownership of the value of the temporary.
1439+
SGF.Cleanups.pushCleanup<TranslateIndirect>(outputOrigType,
1440+
outputSubstType,
1441+
inputOrigType,
1442+
inputSubstType,
1443+
temporaryAddr,
1444+
input.getLValueAddress());
1445+
1446+
// Add the temporary as an l-value argument.
1447+
Outputs.push_back(ManagedValue::forLValue(temporaryAddr));
1448+
return;
13661449
}
13671450
case ParameterConvention::Indirect_In: {
13681451
if (SGF.silConv.useLoweredAddresses()) {

‎lib/SILGen/Scope.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,6 @@ RValue Scope::popPreservingValue(RValue &&rv) {
120120
}
121121

122122
void Scope::popImpl() {
123-
SmallVector<SILValue, 16> cleanupsToPropagateToOuterScope;
124-
125123
cleanups.stack.checkIterator(depth);
126124
cleanups.stack.checkIterator(cleanups.innermostScope);
127125
assert(cleanups.innermostScope == depth && "popping scopes out of order");

‎test/SILGen/witnesses.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,3 +522,37 @@ protocol EscapingReq {
522522
struct EscapingCovariance: EscapingReq {
523523
func f(_: (Int) -> Int) { }
524524
}
525+
526+
protocol InoutFunctionReq {
527+
associatedtype T
528+
func updateFunction(x: inout () -> T)
529+
}
530+
531+
// CHECK-LABEL: sil private [transparent] [thunk] @$S9witnesses13InoutFunctionVAA0bC3ReqA2aDP06updateC01xy1TQzycz_tFTW
532+
// CHECK: bb0(%0 : @trivial $*@callee_guaranteed () -> @out (), %1 : @trivial $*InoutFunction):
533+
// CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $@callee_guaranteed () -> ()
534+
// Reabstract the contents of the inout argument into the temporary.
535+
// CHECK-NEXT: [[OLD_FN:%.*]] = load [take] %0
536+
// CHECK-NEXT: // function_ref
537+
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$SytIegr_Ieg_TR
538+
// CHECK-NEXT: [[THUNKED_OLD_FN:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[OLD_FN]])
539+
// CHECK-NEXT: store [[THUNKED_OLD_FN]] to [init] [[TEMP]] :
540+
// Call the function.
541+
// CHECK-NEXT: [[SELF:%.*]] = load [trivial] %1 : $*InoutFunction
542+
// CHECK-NEXT: // function_ref
543+
// CHECK-NEXT: [[T0:%.*]] = function_ref @$S9witnesses13InoutFunctionV06updateC01xyyycz_tF :
544+
// CHECK-NEXT: apply [[T0]]([[TEMP]], [[SELF]])
545+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
546+
// Reabstract the contents of the temporary back into the inout argument.
547+
// CHECK-NEXT: [[NEW_FN:%.*]] = load [take] [[TEMP]]
548+
// CHECK-NEXT: // function_ref
549+
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$SIeg_ytIegr_TR
550+
// CHECK-NEXT: [[THUNKED_NEW_FN:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[NEW_FN]])
551+
// CHECK-NEXT: store [[THUNKED_NEW_FN]] to [init] %0 :
552+
// CHECK-NEXT: dealloc_stack [[TEMP]]
553+
// CHECK-NEXT: return [[TUPLE]]
554+
// CHECK-LABEL: } // end sil function '$S9witnesses13InoutFunctionVAA0bC3ReqA2aDP06updateC01xy1TQzycz_tFTW'
555+
556+
struct InoutFunction : InoutFunctionReq {
557+
func updateFunction(x: inout () -> ()) {}
558+
}

0 commit comments

Comments
 (0)