Skip to content

Commit d47bfe5

Browse files
committed
Fix some SILArgument infrastructure for pack results.
Getting this right convinces the memory verifier to not complain about untouched empty pack result arguments, which should be innocuous.
1 parent bf9632e commit d47bfe5

File tree

8 files changed

+85
-23
lines changed

8 files changed

+85
-23
lines changed

SwiftCompilerSources/Sources/SIL/Argument.swift

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,17 @@ public enum ArgumentConvention {
137137
/// indirectly is recorded in the pack type.
138138
case packGuaranteed
139139

140+
/// This argument is a pack of indirect return value addresses. The
141+
/// addresses are stored in the pack by the caller and read out by the
142+
/// callee; within the callee, they are individually treated like
143+
/// indirectOut arguments.
144+
case packOut
145+
140146
public var isIndirect: Bool {
141147
switch self {
142148
case .indirectIn, .indirectInGuaranteed,
143149
.indirectInout, .indirectInoutAliasable, .indirectOut,
144-
.packInout, .packOwned, .packGuaranteed:
150+
.packOut, .packInout, .packOwned, .packGuaranteed:
145151
return true
146152
case .directOwned, .directUnowned, .directGuaranteed:
147153
return false
@@ -155,7 +161,19 @@ public enum ArgumentConvention {
155161
return true
156162
case .directOwned, .directUnowned, .directGuaranteed,
157163
.indirectInout, .indirectInoutAliasable, .indirectOut,
158-
.packInout:
164+
.packOut, .packInout:
165+
return false
166+
}
167+
}
168+
169+
public var isIndirectOut: Bool {
170+
switch self {
171+
case .indirectOut, .packOut:
172+
return true
173+
case .indirectInGuaranteed, .directGuaranteed, .packGuaranteed,
174+
.indirectIn, .directOwned, .directUnowned,
175+
.indirectInout, .indirectInoutAliasable,
176+
.packInout, .packOwned:
159177
return false
160178
}
161179
}
@@ -166,7 +184,7 @@ public enum ArgumentConvention {
166184
return true
167185
case .indirectIn, .directOwned, .directUnowned,
168186
.indirectInout, .indirectInoutAliasable, .indirectOut,
169-
.packInout, .packOwned:
187+
.packOut, .packInout, .packOwned:
170188
return false
171189
}
172190
}
@@ -177,6 +195,7 @@ public enum ArgumentConvention {
177195
.indirectOut,
178196
.indirectInGuaranteed,
179197
.indirectInout,
198+
.packOut,
180199
.packInout,
181200
.packOwned,
182201
.packGuaranteed:
@@ -203,6 +222,7 @@ public enum ArgumentConvention {
203222
.directUnowned,
204223
.directGuaranteed,
205224
.directOwned,
225+
.packOut,
206226
.packOwned,
207227
.packGuaranteed:
208228
return false
@@ -229,6 +249,7 @@ extension BridgedArgumentConvention {
229249
case .Direct_Owned: return .directOwned
230250
case .Direct_Unowned: return .directUnowned
231251
case .Direct_Guaranteed: return .directGuaranteed
252+
case .Pack_Out: return .packOut
232253
case .Pack_Inout: return .packInout
233254
case .Pack_Owned: return .packOwned
234255
case .Pack_Guaranteed: return .packGuaranteed

SwiftCompilerSources/Sources/SIL/Effects.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ extension Function {
143143
if convention.isIndirectIn {
144144
// Even a `[readnone]` function can read from an indirect argument.
145145
result.memory.read = true
146-
} else if convention == .indirectOut {
146+
} else if convention.isIndirectOut {
147147
// Even `[readnone]` and `[readonly]` functions write to indirect results.
148148
result.memory.write = true
149149
}
@@ -546,7 +546,7 @@ public struct SideEffects : CustomStringConvertible, NoReflectionChildren {
546546
case .indirectInGuaranteed, .packGuaranteed:
547547
result.memory.write = false
548548
result.ownership.destroy = false
549-
case .indirectOut, .packInout:
549+
case .indirectOut, .packOut, .packInout:
550550
result.memory.read = false
551551
result.ownership.copy = false
552552
result.ownership.destroy = false

SwiftCompilerSources/Sources/SIL/Function.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,6 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
8383
public var resultType: Type { bridged.getSILResultType().type }
8484

8585
public func getArgumentConvention(for argumentIndex: Int) -> ArgumentConvention {
86-
if argumentIndex < numIndirectResultArguments {
87-
return .indirectOut
88-
}
8986
return bridged.getSILArgumentConvention(argumentIndex).convention
9087
}
9188

include/swift/SIL/SILArgument.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,6 @@ class SILPhiArgument;
2828
class SILUndef;
2929
class TermInst;
3030

31-
// Map an argument index onto a SILArgumentConvention.
32-
inline SILArgumentConvention
33-
SILFunctionConventions::getSILArgumentConvention(unsigned index) const {
34-
assert(index <= getNumSILArguments());
35-
if (index < getNumIndirectSILResults()) {
36-
assert(silConv.loweredAddresses);
37-
return SILArgumentConvention::Indirect_Out;
38-
} else {
39-
auto param = funcTy->getParameters()[index - getNumIndirectSILResults()];
40-
return SILArgumentConvention(param.getConvention());
41-
}
42-
}
43-
4431
struct SILArgumentKind {
4532
enum innerty : std::underlying_type<ValueKind>::type {
4633
#define ARGUMENT(ID, PARENT) ID = unsigned(SILNodeKind::ID),

include/swift/SIL/SILArgumentConvention.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,28 @@ struct SILArgumentConvention {
164164
}
165165
llvm_unreachable("covered switch isn't covered?!");
166166
}
167+
168+
/// Returns true if \p Value is an indirect-out parameter.
169+
bool isIndirectOutParameter() {
170+
switch (Value) {
171+
case SILArgumentConvention::Indirect_Out:
172+
case SILArgumentConvention::Pack_Out:
173+
return true;
174+
175+
case SILArgumentConvention::Indirect_In:
176+
case SILArgumentConvention::Indirect_In_Guaranteed:
177+
case SILArgumentConvention::Indirect_Inout:
178+
case SILArgumentConvention::Indirect_InoutAliasable:
179+
case SILArgumentConvention::Direct_Unowned:
180+
case SILArgumentConvention::Direct_Guaranteed:
181+
case SILArgumentConvention::Direct_Owned:
182+
case SILArgumentConvention::Pack_Inout:
183+
case SILArgumentConvention::Pack_Owned:
184+
case SILArgumentConvention::Pack_Guaranteed:
185+
return false;
186+
}
187+
llvm_unreachable("covered switch isn't covered?!");
188+
}
167189
};
168190

169191
} // namespace swift

include/swift/SIL/SILBridging.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ struct BridgedFunction {
212212

213213
BridgedArgumentConvention getSILArgumentConvention(SwiftInt idx) const {
214214
swift::SILFunctionConventions conv(getFunction()->getConventionsInContext());
215-
return castToArgumentConvention(swift::SILArgumentConvention(conv.getParamInfoForSILArg(idx).getConvention()));
215+
return castToArgumentConvention(conv.getSILArgumentConvention(idx));
216216
}
217217

218218
swift::SILType getSILResultType() const {

include/swift/SIL/SILFunctionConventions.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,6 @@ class SILFunctionConventions {
409409
/// Return the SIL argument convention of apply/entry argument at
410410
/// the given argument index.
411411
SILArgumentConvention getSILArgumentConvention(unsigned index) const;
412-
// See SILArgument.h.
413412

414413
/// Return the SIL type of the apply/entry argument at the given index.
415414
SILType getSILArgumentType(unsigned index,

lib/SIL/IR/SILArgument.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,42 @@ SILParameterInfo SILFunctionArgument::getKnownParameterInfo() const {
7272
return getFunction()->getConventions().getParamInfoForSILArg(getIndex());
7373
}
7474

75+
SILArgumentConvention
76+
SILFunctionConventions::getSILArgumentConvention(unsigned index) const {
77+
assert(index < getNumSILArguments());
78+
79+
// If the argument is a parameter, index into the parameters.
80+
if (index >= getNumIndirectSILResults()) {
81+
auto param = funcTy->getParameters()[index - getNumIndirectSILResults()];
82+
return SILArgumentConvention(param.getConvention());
83+
}
84+
85+
// If it's an indirect result, it could be either Pack_Out or
86+
// Indirect_Out.
87+
88+
// Handle the common case of a function with no pack results.
89+
if (funcTy->getNumPackResults() == 0) {
90+
assert(silConv.loweredAddresses);
91+
return SILArgumentConvention::Indirect_Out;
92+
}
93+
94+
// Otherwise, we need to index into the indirect results to figure out
95+
// whether the result is a pack or not, and unfortunately that is not a
96+
// linear algorithm.
97+
for (auto result : getIndirectSILResults()) {
98+
if (index == 0) {
99+
if (result.getConvention() == ResultConvention::Indirect) {
100+
assert(silConv.loweredAddresses);
101+
return SILArgumentConvention::Indirect_Out;
102+
} else {
103+
assert(result.getConvention() == ResultConvention::Pack);
104+
return SILArgumentConvention::Pack_Out;
105+
}
106+
}
107+
index--;
108+
}
109+
llvm_unreachable("mismatch with getNumIndirectSILResults()?");
110+
}
75111

76112
//===----------------------------------------------------------------------===//
77113
// SILBlockArgument

0 commit comments

Comments
 (0)