Skip to content

Commit 82744aa

Browse files
committed
Address review comments
1 parent 337f7dd commit 82744aa

File tree

4 files changed

+22
-69
lines changed

4 files changed

+22
-69
lines changed

docs/SIL.rst

+2-2
Original file line numberDiff line numberDiff line change
@@ -6093,8 +6093,8 @@ when the coroutine reaches a ``return`` instruction.
60936093
The operand must always be the token result of a ``begin_apply``
60946094
instruction, which is why it need not specify a type.
60956095

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

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

lib/SIL/IR/ValueOwnership.cpp

+15-29
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

+2-12
Original file line numberDiff line numberDiff line change
@@ -2640,20 +2640,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
26402640
}
26412641
case SILInstructionKind::EndApplyInst: {
26422642
UnresolvedValueName argName;
2643-
if (parseValueName(argName))
2644-
return true;
2645-
2646-
if (P.Tok.getKind() != tok::kw_as) {
2647-
P.diagnose(P.Tok, diag::expected_tok_in_sil_instr, "as");
2648-
return true;
2649-
}
2650-
P.consumeToken(tok::kw_as);
2651-
26522643
SILType ResultTy;
2653-
if (parseSILType(ResultTy))
2654-
return true;
26552644

2656-
if (parseSILDebugLocation(InstLoc, B))
2645+
if (parseValueName(argName) || parseVerbatim("as") ||
2646+
parseSILType(ResultTy) || parseSILDebugLocation(InstLoc, B))
26572647
return true;
26582648

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

lib/SIL/Verifier/SILVerifier.cpp

+3-26
Original file line numberDiff line numberDiff line change
@@ -1935,34 +1935,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
19351935
"operand of end_apply must be a begin_apply");
19361936

19371937
BeginApplyInst *bai = AI->getBeginApply();
1938-
19391938
SILFunctionConventions calleeConv(bai->getSubstCalleeType(), F.getModule());
1940-
auto calleeResults = calleeConv.getResults();
1941-
auto results = AI->getResults();
1942-
1943-
require(results.size() == 1,
1944-
"end_apply must have a single result");
1945-
1946-
if (auto tupleTy = results[0]->getType().getAs<TupleType>()) {
1947-
require(tupleTy.getElementTypes().size() == calleeResults.size(),
1948-
"length mismatch in callee results vs. end_apply results");
1949-
1950-
for (auto typeAndIdx : llvm::enumerate(tupleTy->getElementTypes())) {
1951-
SILType elTy = SILType::getPrimitiveObjectType(typeAndIdx.value()->getCanonicalType());
1952-
requireSameType(
1953-
elTy,
1954-
calleeConv.getSILType(calleeResults[typeAndIdx.index()], F.getTypeExpansionContext()),
1955-
"callee result type does not match end_apply result type");
1956-
}
1957-
} else {
1958-
require(calleeResults.size() == 1,
1959-
"callee must have a single result");
19601939

1961-
requireSameType(
1962-
results[0]->getType(),
1963-
calleeConv.getSILType(calleeResults[0], F.getTypeExpansionContext()),
1964-
"callee result type does not match end_apply result type");
1965-
}
1940+
requireSameType(
1941+
AI->getType(), calleeConv.getSILResultType(F.getTypeExpansionContext()),
1942+
"callee result type does not match end_apply result type");
19661943
}
19671944

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

0 commit comments

Comments
 (0)