Skip to content

Commit aae8931

Browse files
committed
Address review comments
1 parent 924e363 commit aae8931

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
@@ -6149,8 +6149,8 @@ when the coroutine reaches a ``return`` instruction.
61496149
The operand must always be the token result of a ``begin_apply``
61506150
instruction, which is why it need not specify a type.
61516151

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

61556155
When throwing coroutines are supported, there will need to be a
61566156
``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/SILBuiltinVisitor.h"
1415
#include "swift/SIL/SILModule.h"
1516
#include "swift/SIL/SILVisitor.h"
@@ -348,14 +349,18 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitSILFunctionArgument(
348349
return Arg->getOwnershipKind();
349350
}
350351

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

358-
SILFunctionConventions fnConv(ai->getSubstCalleeType(), f->getModule());
363+
SILFunctionConventions fnConv(fai.getSubstCalleeType(), f->getModule());
359364
auto results = fnConv.getDirectSILResults();
360365
// No results => None.
361366
if (results.empty())
@@ -364,7 +369,7 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
364369
// Otherwise, map our results to their ownership kinds and then merge them!
365370
auto resultOwnershipKinds =
366371
makeTransformRange(results, [&](const SILResultInfo &info) {
367-
return info.getOwnershipKind(*f, ai->getSubstCalleeType());
372+
return info.getOwnershipKind(*f, fai.getSubstCalleeType());
368373
});
369374
auto mergedOwnershipKind = ValueOwnershipKind::merge(resultOwnershipKinds);
370375
if (!mergedOwnershipKind) {
@@ -374,31 +379,12 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
374379
return mergedOwnershipKind;
375380
}
376381

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

401-
return mergedOwnershipKind;
386+
ValueOwnershipKind ValueOwnershipKindClassifier::visitEndApplyInst(EndApplyInst *eai) {
387+
return visitFullApplySite(eai->getBeginApply(), eai->getType());
402388
}
403389

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

lib/SIL/Parser/ParseSIL.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,20 +2667,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
26672667
}
26682668
case SILInstructionKind::EndApplyInst: {
26692669
UnresolvedValueName argName;
2670-
if (parseValueName(argName))
2671-
return true;
2672-
2673-
if (P.Tok.getKind() != tok::kw_as) {
2674-
P.diagnose(P.Tok, diag::expected_tok_in_sil_instr, "as");
2675-
return true;
2676-
}
2677-
P.consumeToken(tok::kw_as);
2678-
26792670
SILType ResultTy;
2680-
if (parseSILType(ResultTy))
2681-
return true;
26822671

2683-
if (parseSILDebugLocation(InstLoc, B))
2672+
if (parseValueName(argName) || parseVerbatim("as") ||
2673+
parseSILType(ResultTy) || parseSILDebugLocation(InstLoc, B))
26842674
return true;
26852675

26862676
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
@@ -2033,34 +2033,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
20332033
"operand of end_apply must be a begin_apply");
20342034

20352035
BeginApplyInst *bai = AI->getBeginApply();
2036-
20372036
SILFunctionConventions calleeConv(bai->getSubstCalleeType(), F.getModule());
2038-
auto calleeResults = calleeConv.getResults();
2039-
auto results = AI->getResults();
2040-
2041-
require(results.size() == 1,
2042-
"end_apply must have a single result");
2043-
2044-
if (auto tupleTy = results[0]->getType().getAs<TupleType>()) {
2045-
require(tupleTy.getElementTypes().size() == calleeResults.size(),
2046-
"length mismatch in callee results vs. end_apply results");
2047-
2048-
for (auto typeAndIdx : llvm::enumerate(tupleTy->getElementTypes())) {
2049-
SILType elTy = SILType::getPrimitiveObjectType(typeAndIdx.value()->getCanonicalType());
2050-
requireSameType(
2051-
elTy,
2052-
calleeConv.getSILType(calleeResults[typeAndIdx.index()], F.getTypeExpansionContext()),
2053-
"callee result type does not match end_apply result type");
2054-
}
2055-
} else {
2056-
require(calleeResults.size() == 1,
2057-
"callee must have a single result");
20582037

2059-
requireSameType(
2060-
results[0]->getType(),
2061-
calleeConv.getSILType(calleeResults[0], F.getTypeExpansionContext()),
2062-
"callee result type does not match end_apply result type");
2063-
}
2038+
requireSameType(
2039+
AI->getType(), calleeConv.getSILResultType(F.getTypeExpansionContext()),
2040+
"callee result type does not match end_apply result type");
20642041
}
20652042

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

0 commit comments

Comments
 (0)