Skip to content

Commit 229eea4

Browse files
committed
Address review comments
1 parent 34fc75f commit 229eea4

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

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

61446144
When throwing coroutines are supported, there will need to be a
61456145
``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"
@@ -347,14 +348,18 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitSILFunctionArgument(
347348
return Arg->getOwnershipKind();
348349
}
349350

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

357-
SILFunctionConventions fnConv(ai->getSubstCalleeType(), f->getModule());
362+
SILFunctionConventions fnConv(fai.getSubstCalleeType(), f->getModule());
358363
auto results = fnConv.getDirectSILResults();
359364
// No results => None.
360365
if (results.empty())
@@ -363,7 +368,7 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
363368
// Otherwise, map our results to their ownership kinds and then merge them!
364369
auto resultOwnershipKinds =
365370
makeTransformRange(results, [&](const SILResultInfo &info) {
366-
return info.getOwnershipKind(*f, ai->getSubstCalleeType());
371+
return info.getOwnershipKind(*f, fai.getSubstCalleeType());
367372
});
368373
auto mergedOwnershipKind = ValueOwnershipKind::merge(resultOwnershipKinds);
369374
if (!mergedOwnershipKind) {
@@ -373,31 +378,12 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
373378
return mergedOwnershipKind;
374379
}
375380

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

400-
return mergedOwnershipKind;
385+
ValueOwnershipKind ValueOwnershipKindClassifier::visitEndApplyInst(EndApplyInst *eai) {
386+
return visitFullApplySite(eai->getBeginApply(), eai->getType());
401387
}
402388

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

lib/SIL/Parser/ParseSIL.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2636,20 +2636,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
26362636
}
26372637
case SILInstructionKind::EndApplyInst: {
26382638
UnresolvedValueName argName;
2639-
if (parseValueName(argName))
2640-
return true;
2641-
2642-
if (P.Tok.getKind() != tok::kw_as) {
2643-
P.diagnose(P.Tok, diag::expected_tok_in_sil_instr, "as");
2644-
return true;
2645-
}
2646-
P.consumeToken(tok::kw_as);
2647-
26482639
SILType ResultTy;
2649-
if (parseSILType(ResultTy))
2650-
return true;
26512640

2652-
if (parseSILDebugLocation(InstLoc, B))
2641+
if (parseValueName(argName) || parseVerbatim("as") ||
2642+
parseSILType(ResultTy) || parseSILDebugLocation(InstLoc, B))
26532643
return true;
26542644

26552645
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)