Skip to content

Commit 89ed064

Browse files
committed
Fix several incorrect uses of ApplySite::getArgumentConvention.
At least most of these were latent bugs since the code was unreachable in the PartialApply case. But that's no excuse to misuse the API. Also, whenever referring to an integer index, be explicit about whether it is an applied argument or callee argument.
1 parent 401be14 commit 89ed064

File tree

7 files changed

+19
-39
lines changed

7 files changed

+19
-39
lines changed

Diff for: include/swift/SIL/SILInstruction.h

+3-13
Original file line numberDiff line numberDiff line change
@@ -1936,11 +1936,6 @@ class ApplyInstBase<Impl, Base, true>
19361936
return OperandValueArrayRef(opsWithoutSelf);
19371937
}
19381938

1939-
/// Return the SILArgumentConvention for the given applied argument index.
1940-
SILArgumentConvention getArgumentConvention(unsigned index) const {
1941-
return getSubstCalleeConv().getSILArgumentConvention(index);
1942-
}
1943-
19441939
Optional<SILResultInfo> getSingleResult() const {
19451940
auto SubstCallee = getSubstCalleeType();
19461941
if (SubstCallee->getNumAllResults() != 1)
@@ -7665,7 +7660,7 @@ class ApplySite {
76657660
}
76667661

76677662
/// Return the applied argument index for the given operand.
7668-
unsigned getArgumentIndex(const Operand &oper) const {
7663+
unsigned getAppliedArgIndex(const Operand &oper) const {
76697664
assert(oper.getUser() == Inst);
76707665
assert(isArgumentOperand(oper));
76717666

@@ -7704,21 +7699,16 @@ class ApplySite {
77047699
/// Note: Passing an applied argument index into SILFunctionConvention, as
77057700
/// opposed to a function argument index, is incorrect.
77067701
unsigned getCalleeArgIndex(const Operand &oper) const {
7707-
return getCalleeArgIndexOfFirstAppliedArg() + getArgumentIndex(oper);
7702+
return getCalleeArgIndexOfFirstAppliedArg() + getAppliedArgIndex(oper);
77087703
}
77097704

77107705
/// Return the SILArgumentConvention for the given applied argument operand.
77117706
SILArgumentConvention getArgumentConvention(Operand &oper) const {
77127707
unsigned calleeArgIdx =
7713-
getCalleeArgIndexOfFirstAppliedArg() + getArgumentIndex(oper);
7708+
getCalleeArgIndexOfFirstAppliedArg() + getAppliedArgIndex(oper);
77147709
return getSubstCalleeConv().getSILArgumentConvention(calleeArgIdx);
77157710
}
77167711

7717-
// FIXME: This is incorrect. It will be removed in the next commit.
7718-
SILArgumentConvention getArgumentConvention(unsigned index) const {
7719-
return getSubstCalleeConv().getSILArgumentConvention(index);
7720-
}
7721-
77227712
/// Return true if 'self' is an applied argument.
77237713
bool hasSelfArgument() const {
77247714
switch (Inst->getKind()) {

Diff for: lib/SIL/SILVerifier.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2713,7 +2713,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
27132713
auto isConsumingOrMutatingApplyUse = [](Operand *use) -> bool {
27142714
ApplySite apply(use->getUser());
27152715
assert(apply && "Not an apply instruction kind");
2716-
auto conv = apply.getArgumentConvention(use->getOperandNumber() - 1);
2716+
auto conv = apply.getArgumentConvention(*use);
27172717
switch (conv) {
27182718
case SILArgumentConvention::Indirect_In_Guaranteed:
27192719
return false;

Diff for: lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,7 @@ static SILInstruction *getOnlyDestroy(CopyBlockWithoutEscapingInst *CB) {
339339

340340
// If this an apply use, only handle unowned parameters.
341341
if (auto Apply = FullApplySite::isa(Inst)) {
342-
SILArgumentConvention Conv =
343-
Apply.getArgumentConvention(Apply.getCalleeArgIndex(*Use));
342+
SILArgumentConvention Conv = Apply.getArgumentConvention(*Use);
344343
if (Conv != SILArgumentConvention::Direct_Unowned)
345344
return nullptr;
346345
continue;

Diff for: lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

+5-9
Original file line numberDiff line numberDiff line change
@@ -877,13 +877,10 @@ static void checkForViolationsAtInstruction(SILInstruction &I,
877877
// SILVerifier to better pinpoint the offending pass.
878878
if (auto *PAI = dyn_cast<PartialApplyInst>(&I)) {
879879
ApplySite apply(PAI);
880-
if (llvm::any_of(range(apply.getNumArguments()),
881-
[apply](unsigned argIdx) {
882-
unsigned calleeIdx =
883-
apply.getCalleeArgIndexOfFirstAppliedArg() + argIdx;
884-
return apply.getArgumentConvention(calleeIdx)
885-
== SILArgumentConvention::Indirect_InoutAliasable;
886-
})) {
880+
if (llvm::any_of(apply.getArgumentOperands(), [apply](Operand &oper) {
881+
return apply.getArgumentConvention(oper)
882+
== SILArgumentConvention::Indirect_InoutAliasable;
883+
})) {
887884
checkNoEscapePartialApply(PAI);
888885
}
889886
}
@@ -1071,8 +1068,7 @@ static void checkAccessedAddress(Operand *memOper, StorageMap &Accesses) {
10711068
return;
10721069

10731070
if (auto apply = ApplySite::isa(memInst)) {
1074-
SILArgumentConvention conv =
1075-
apply.getArgumentConvention(apply.getCalleeArgIndex(*memOper));
1071+
SILArgumentConvention conv = apply.getArgumentConvention(*memOper);
10761072
// Captured addresses currently use the @inout_aliasable convention. They
10771073
// are considered an access at any call site that uses the closure. However,
10781074
// those accesses are never explictly protected by access markers. Instead,

Diff for: lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ static bool partialApplyEscapes(SILValue V, bool examineApply) {
284284
SILModuleConventions ModConv(*V->getModule());
285285
llvm::SmallVector<Operand *, 32> Worklist(V->use_begin(), V->use_end());
286286
while (!Worklist.empty()) {
287-
auto *Op = Worklist.pop_back_val();
287+
Operand *Op = Worklist.pop_back_val();
288288

289289
// These instructions do not cause the address to escape.
290290
if (!useCaptured(Op))
@@ -300,15 +300,14 @@ static bool partialApplyEscapes(SILValue V, bool examineApply) {
300300
continue;
301301
}
302302

303-
if (auto *Apply = dyn_cast<ApplyInst>(User)) {
303+
if (auto Apply = FullApplySite::isa(User)) {
304304
// Applying a function does not cause the function to escape.
305-
if (Op->getOperandNumber() == 0)
305+
if (!Apply.isArgumentOperand(*Op))
306306
continue;
307307

308308
// apply instructions do not capture the pointer when it is passed
309309
// indirectly
310-
if (Apply->getArgumentConvention(Op->getOperandNumber() - 1)
311-
.isIndirectConvention())
310+
if (Apply.getArgumentConvention(*Op).isIndirectConvention())
312311
continue;
313312

314313
// Optionally drill down into an apply to see if the operand is

Diff for: lib/SILOptimizer/Transforms/CopyForwarding.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -152,20 +152,18 @@ static SILArgumentConvention getAddressArgConvention(ApplyInst *Apply,
152152
Operand *&Oper) {
153153
Oper = nullptr;
154154
auto Args = Apply->getArgumentOperands();
155-
llvm::Optional<unsigned> FoundArgIdx;
156155
for (auto ArgIdx : indices(Args)) {
157156
if (Args[ArgIdx].get() != Address)
158157
continue;
159158

160-
FoundArgIdx = ArgIdx;
161159
assert(!Oper && "Address can only be passed once as an indirection.");
162160
Oper = &Args[ArgIdx];
163161
#ifdef NDEBUG
164162
break;
165163
#endif
166164
}
167165
assert(Oper && "Address value not passed as an argument to this call.");
168-
return Apply->getArgumentConvention(FoundArgIdx.getValue());
166+
return ApplySite(Apply).getArgumentConvention(*Oper);
169167
}
170168

171169
/// If the given instruction is a store, return the stored value.
@@ -1633,10 +1631,10 @@ bool TempRValueOptPass::collectLoads(
16331631
return false;
16341632

16351633
case SILInstructionKind::ApplyInst: {
1636-
auto *AI = cast<ApplyInst>(user);
1637-
auto Convention = AI->getArgumentConvention(userOp->getOperandNumber() - 1);
1634+
ApplySite apply(user);
1635+
auto Convention = apply.getArgumentConvention(*userOp);
16381636
if (Convention.isGuaranteedConvention()) {
1639-
loadInsts.insert(AI);
1637+
loadInsts.insert(user);
16401638
return true;
16411639
}
16421640
LLVM_DEBUG(llvm::dbgs() << " Temp consuming use may write/destroy "

Diff for: lib/SILOptimizer/Utils/Existential.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,7 @@ SILValue swift::getAddressOfStackInit(AllocStackInst *ASI,
152152
}
153153
if (isa<ApplyInst>(User) || isa<TryApplyInst>(User)) {
154154
// Ignore function calls which do not write to the stack location.
155-
auto Idx =
156-
Use->getOperandNumber() - ApplyInst::getArgumentOperandNumber();
157-
auto Conv = FullApplySite(User).getArgumentConvention(Idx);
155+
auto Conv = FullApplySite(User).getArgumentConvention(*Use);
158156
if (Conv != SILArgumentConvention::Indirect_In &&
159157
Conv != SILArgumentConvention::Indirect_In_Guaranteed)
160158
return SILValue();

0 commit comments

Comments
 (0)