Skip to content

Commit b06b0db

Browse files
committed
Address review comments
1 parent 7ab070b commit b06b0db

File tree

4 files changed

+22
-69
lines changed

4 files changed

+22
-69
lines changed

docs/SIL.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6116,8 +6116,8 @@ when the coroutine reaches a ``return`` instruction.
61166116
The operand must always be the token result of a ``begin_apply``
61176117
instruction, which is why it need not specify a type.
61186118

6119-
If a coroutine produces normal results on ``resume`` path, they
6120-
will be produced by ``end_apply``.
6119+
The result of ``end_apply`` is the normal result of the coroutine function (the
6120+
operand of the ``return`` instruction)."
61216121

61226122
When throwing coroutines are supported, there will need to be a
61236123
``try_end_apply`` instruction.

lib/SIL/IR/ValueOwnership.cpp

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "swift/SIL/ApplySite.h"
1314
#include "swift/SIL/SILVisitor.h"
1415
#include "swift/SIL/SILBuiltinVisitor.h"
1516
#include "swift/SIL/SILModule.h"
@@ -346,14 +347,18 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitSILFunctionArgument(
346347
return Arg->getOwnershipKind();
347348
}
348349

349-
ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
350-
auto *f = ai->getFunction();
351-
bool isTrivial = ai->getType().isTrivial(*f);
350+
// We have to separate out ResultType here as `begin_apply` does not produce
351+
// normal results, `end_apply` does and there might be multiple `end_apply`'s
352+
// that correspond to a single `begin_apply`.
353+
static ValueOwnershipKind visitFullApplySite(FullApplySite fai,
354+
SILType ResultType) {
355+
auto *f = fai->getFunction();
356+
bool isTrivial = ResultType.isTrivial(*f);
352357
// Quick is trivial check.
353358
if (isTrivial)
354359
return OwnershipKind::None;
355360

356-
SILFunctionConventions fnConv(ai->getSubstCalleeType(), f->getModule());
361+
SILFunctionConventions fnConv(fai.getSubstCalleeType(), f->getModule());
357362
auto results = fnConv.getDirectSILResults();
358363
// No results => None.
359364
if (results.empty())
@@ -362,7 +367,7 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
362367
// Otherwise, map our results to their ownership kinds and then merge them!
363368
auto resultOwnershipKinds =
364369
makeTransformRange(results, [&](const SILResultInfo &info) {
365-
return info.getOwnershipKind(*f, ai->getSubstCalleeType());
370+
return info.getOwnershipKind(*f, fai.getSubstCalleeType());
366371
});
367372
auto mergedOwnershipKind = ValueOwnershipKind::merge(resultOwnershipKinds);
368373
if (!mergedOwnershipKind) {
@@ -372,31 +377,12 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
372377
return mergedOwnershipKind;
373378
}
374379

375-
ValueOwnershipKind ValueOwnershipKindClassifier::visitEndApplyInst(EndApplyInst *eai) {
376-
auto *bai = eai->getBeginApply();
377-
auto *f = bai->getFunction();
378-
bool isTrivial = eai->getType().isTrivial(*f);
379-
// Quick is trivial check.
380-
if (isTrivial)
381-
return OwnershipKind::None;
382-
383-
SILFunctionConventions fnConv(bai->getSubstCalleeType(), f->getModule());
384-
auto results = fnConv.getDirectSILResults();
385-
// No results => None.
386-
if (results.empty())
387-
return OwnershipKind::None;
388-
389-
// Otherwise, map our results to their ownership kinds and then merge them!
390-
auto resultOwnershipKinds =
391-
makeTransformRange(results, [&](const SILResultInfo &info) {
392-
return info.getOwnershipKind(*f, bai->getSubstCalleeType());
393-
});
394-
auto mergedOwnershipKind = ValueOwnershipKind::merge(resultOwnershipKinds);
395-
if (!mergedOwnershipKind) {
396-
llvm_unreachable("Forwarding inst with mismatching ownership kinds?!");
397-
}
380+
ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
381+
return visitFullApplySite(ai, ai->getType());
382+
}
398383

399-
return mergedOwnershipKind;
384+
ValueOwnershipKind ValueOwnershipKindClassifier::visitEndApplyInst(EndApplyInst *eai) {
385+
return visitFullApplySite(eai->getBeginApply(), eai->getType());
400386
}
401387

402388
ValueOwnershipKind ValueOwnershipKindClassifier::visitLoadInst(LoadInst *LI) {

lib/SIL/Parser/ParseSIL.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,20 +2649,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
26492649
}
26502650
case SILInstructionKind::EndApplyInst: {
26512651
UnresolvedValueName argName;
2652-
if (parseValueName(argName))
2653-
return true;
2654-
2655-
if (P.Tok.getKind() != tok::kw_as) {
2656-
P.diagnose(P.Tok, diag::expected_tok_in_sil_instr, "as");
2657-
return true;
2658-
}
2659-
P.consumeToken(tok::kw_as);
2660-
26612652
SILType ResultTy;
2662-
if (parseSILType(ResultTy))
2663-
return true;
26642653

2665-
if (parseSILDebugLocation(InstLoc, B))
2654+
if (parseValueName(argName) || parseVerbatim("as") ||
2655+
parseSILType(ResultTy) || parseSILDebugLocation(InstLoc, B))
26662656
return true;
26672657

26682658
SILType expectedTy = SILType::getSILTokenType(P.Context);

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,34 +1951,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
19511951
"operand of end_apply must be a begin_apply");
19521952

19531953
BeginApplyInst *bai = AI->getBeginApply();
1954-
19551954
SILFunctionConventions calleeConv(bai->getSubstCalleeType(), F.getModule());
1956-
auto calleeResults = calleeConv.getResults();
1957-
auto results = AI->getResults();
1958-
1959-
require(results.size() == 1,
1960-
"end_apply must have a single result");
1961-
1962-
if (auto tupleTy = results[0]->getType().getAs<TupleType>()) {
1963-
require(tupleTy.getElementTypes().size() == calleeResults.size(),
1964-
"length mismatch in callee results vs. end_apply results");
1965-
1966-
for (auto typeAndIdx : llvm::enumerate(tupleTy->getElementTypes())) {
1967-
SILType elTy = SILType::getPrimitiveObjectType(typeAndIdx.value()->getCanonicalType());
1968-
requireSameType(
1969-
elTy,
1970-
calleeConv.getSILType(calleeResults[typeAndIdx.index()], F.getTypeExpansionContext()),
1971-
"callee result type does not match end_apply result type");
1972-
}
1973-
} else {
1974-
require(calleeResults.size() == 1,
1975-
"callee must have a single result");
19761955

1977-
requireSameType(
1978-
results[0]->getType(),
1979-
calleeConv.getSILType(calleeResults[0], F.getTypeExpansionContext()),
1980-
"callee result type does not match end_apply result type");
1981-
}
1956+
requireSameType(
1957+
AI->getType(), calleeConv.getSILResultType(F.getTypeExpansionContext()),
1958+
"callee result type does not match end_apply result type");
19821959
}
19831960

19841961
void verifyLLVMIntrinsic(BuiltinInst *BI, llvm::Intrinsic::ID ID) {

0 commit comments

Comments
 (0)