Skip to content

Commit 5383b7e

Browse files
ktosoktoso
authored
Merge branch 'main' into wip-remove-accessor-protocol-sections
2 parents 0d80c8b + 7f9b184 commit 5383b7e

File tree

9 files changed

+256
-46
lines changed

9 files changed

+256
-46
lines changed

‎lib/IRGen/LoadableByAddress.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3306,7 +3306,7 @@ bool Peepholes::optimizeLoad(SILBasicBlock &BB, SILInstruction *I) {
33063306
if (next2It == BB.end())
33073307
return false;
33083308
auto *store = dyn_cast<StoreInst>(&*next2It);
3309-
if (!store)
3309+
if (!store || store->getSrc() != LI)
33103310
return false;
33113311
if (ignore(store))
33123312
return false;
@@ -3459,6 +3459,19 @@ class AddressAssignment {
34593459

34603460
SILValue getAddressForValue(SILValue v) {
34613461
auto it = valueToAddressMap.find(v);
3462+
3463+
// This can happen if we deem a container type small but a contained type
3464+
// big.
3465+
if (it == valueToAddressMap.end()) {
3466+
if (auto *sv = dyn_cast<SingleValueInstruction>(v)) {
3467+
auto addr = createAllocStack(v->getType());
3468+
auto builder = getBuilder(++sv->getIterator());
3469+
builder.createStore(sv->getLoc(), v, addr,
3470+
StoreOwnershipQualifier::Unqualified);
3471+
mapValueToAddress(v, addr);
3472+
return addr;
3473+
}
3474+
}
34623475
assert(it != valueToAddressMap.end());
34633476

34643477
return it->second;

‎lib/SILGen/SILGen.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
418418
/// Emit a global initialization.
419419
void emitGlobalInitialization(PatternBindingDecl *initializer, unsigned elt);
420420

421-
/// Should the self argument of the given method always be emitted as
422-
/// an r-value (meaning that it can be borrowed only if that is not
423-
/// semantically detectable), or it acceptable to emit it as a borrowed
424-
/// storage reference?
425-
bool shouldEmitSelfAsRValue(FuncDecl *method, CanType selfType);
426-
427421
/// Is the self method of the given nonmutating method passed indirectly?
428422
bool isNonMutatingSelfIndirect(SILDeclRef method);
429423

‎lib/SILGen/SILGenApply.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5434,22 +5434,6 @@ CallEmission CallEmission::forApplyExpr(SILGenFunction &SGF, ApplyExpr *e) {
54345434
return emission;
54355435
}
54365436

5437-
bool SILGenModule::shouldEmitSelfAsRValue(FuncDecl *fn, CanType selfType) {
5438-
if (fn->isStatic())
5439-
return true;
5440-
5441-
switch (fn->getSelfAccessKind()) {
5442-
case SelfAccessKind::Mutating:
5443-
return false;
5444-
case SelfAccessKind::NonMutating:
5445-
case SelfAccessKind::LegacyConsuming:
5446-
case SelfAccessKind::Consuming:
5447-
case SelfAccessKind::Borrowing:
5448-
return true;
5449-
}
5450-
llvm_unreachable("bad self-access kind");
5451-
}
5452-
54535437
bool SILGenModule::isNonMutatingSelfIndirect(SILDeclRef methodRef) {
54545438
auto method = methodRef.getFuncDecl();
54555439
if (method->isStatic())

‎lib/SILGen/SILGenFunction.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ enum class SGFAccessKind : uint8_t {
111111
/// The access is a read that would prefer the address of a borrowed value.
112112
/// This should only be used when it is semantically acceptable to borrow
113113
/// the value, not just because the caller would benefit from a borrowed
114-
/// value. See shouldEmitSelfAsRValue.
114+
/// value. See shouldEmitSelfAsRValue in SILGenLValue.cpp.
115115
///
116116
/// The caller will be calling emitAddressOfLValue or emitLoadOfLValue
117117
/// on the l-value. The latter may be less efficient than an access
@@ -121,7 +121,7 @@ enum class SGFAccessKind : uint8_t {
121121
/// The access is a read that would prefer a loaded borrowed value.
122122
/// This should only be used when it is semantically acceptable to borrow
123123
/// the value, not just because the caller would benefit from a borrowed
124-
/// value. See shouldEmitSelfAsRValue.
124+
/// value. See shouldEmitSelfAsRValue in SILGenLValue.cpp.
125125
///
126126
/// There isn't yet a way to emit the access that takes advantage of this.
127127
BorrowedObjectRead,

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,8 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
13381338
AbstractStorageDecl *member,
13391339
SGFAccessKind accessKind,
13401340
AccessStrategy strategy,
1341-
CanType baseFormalType);
1341+
CanType baseFormalType,
1342+
bool forBorrowExpr);
13421343

13431344
namespace {
13441345
/// A helper class for implementing components that involve accessing
@@ -1971,7 +1972,8 @@ namespace {
19711972
if (!base) return LValue();
19721973
auto baseAccessKind =
19731974
getBaseAccessKind(SGF.SGM, Storage, accessKind, strategy,
1974-
BaseFormalType);
1975+
BaseFormalType,
1976+
/*for borrow*/ false);
19751977
return LValue::forValue(baseAccessKind, base, BaseFormalType);
19761978
}();
19771979

@@ -3049,7 +3051,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
30493051
auto baseFormalType = getBaseFormalType(e->getBase());
30503052
LValue lv = visit(
30513053
e->getBase(),
3052-
getBaseAccessKind(SGF.SGM, var, accessKind, strategy, baseFormalType),
3054+
getBaseAccessKind(SGF.SGM, var, accessKind, strategy, baseFormalType,
3055+
/*for borrow*/ true),
30533056
getBaseOptions(options, strategy));
30543057
std::optional<ActorIsolation> actorIso;
30553058
if (e->isImplicitlyAsync())
@@ -3717,14 +3720,50 @@ LValue SILGenLValue::visitDotSyntaxBaseIgnoredExpr(DotSyntaxBaseIgnoredExpr *e,
37173720
return visitRec(e->getRHS(), accessKind, options);
37183721
}
37193722

3723+
/// Should the self argument of the given method always be emitted as
3724+
/// an r-value (meaning that it can be borrowed only if that is not
3725+
/// semantically detectable), or it acceptable to emit it as a borrowed
3726+
/// storage reference?
3727+
static bool shouldEmitSelfAsRValue(AccessorDecl *fn, CanType selfType,
3728+
bool forBorrowExpr) {
3729+
if (fn->isStatic())
3730+
return true;
3731+
3732+
switch (fn->getSelfAccessKind()) {
3733+
case SelfAccessKind::Mutating:
3734+
return false;
3735+
case SelfAccessKind::Borrowing:
3736+
case SelfAccessKind::NonMutating:
3737+
// If the accessor is a coroutine, we may want to access the projected
3738+
// value through a borrow of the base. But if it's a regular get/set then
3739+
// there isn't any real benefit to doing so.
3740+
if (!fn->isCoroutine()) {
3741+
return true;
3742+
}
3743+
// Normally we'll copy the base to minimize accesses. But if the base
3744+
// is noncopyable, or we're accessing it in a `borrow` expression, then
3745+
// we want to keep the access nested on the original base.
3746+
if (forBorrowExpr || selfType->isNoncopyable()) {
3747+
return false;
3748+
}
3749+
return true;
3750+
3751+
case SelfAccessKind::LegacyConsuming:
3752+
case SelfAccessKind::Consuming:
3753+
return true;
3754+
}
3755+
llvm_unreachable("bad self-access kind");
3756+
}
3757+
37203758
static SGFAccessKind getBaseAccessKindForAccessor(SILGenModule &SGM,
37213759
AccessorDecl *accessor,
3722-
CanType baseFormalType) {
3760+
CanType baseFormalType,
3761+
bool forBorrowExpr) {
37233762
if (accessor->isMutating())
37243763
return SGFAccessKind::ReadWrite;
37253764

37263765
auto declRef = SGM.getAccessorDeclRef(accessor, ResilienceExpansion::Minimal);
3727-
if (SGM.shouldEmitSelfAsRValue(accessor, baseFormalType)) {
3766+
if (shouldEmitSelfAsRValue(accessor, baseFormalType, forBorrowExpr)) {
37283767
return SGM.isNonMutatingSelfIndirect(declRef)
37293768
? SGFAccessKind::OwnedAddressRead
37303769
: SGFAccessKind::OwnedObjectRead;
@@ -3748,7 +3787,8 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
37483787
AbstractStorageDecl *member,
37493788
SGFAccessKind accessKind,
37503789
AccessStrategy strategy,
3751-
CanType baseFormalType) {
3790+
CanType baseFormalType,
3791+
bool forBorrowExpr) {
37523792
switch (strategy.getKind()) {
37533793
case AccessStrategy::Storage:
37543794
return getBaseAccessKindForStorage(accessKind);
@@ -3757,7 +3797,8 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
37573797
assert(accessKind == SGFAccessKind::ReadWrite);
37583798
auto writeBaseKind = getBaseAccessKind(SGM, member, SGFAccessKind::Write,
37593799
strategy.getWriteStrategy(),
3760-
baseFormalType);
3800+
baseFormalType,
3801+
/*for borrow*/ false);
37613802

37623803
// Fast path for the common case that the write will need to mutate
37633804
// the base.
@@ -3767,7 +3808,8 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
37673808
auto readBaseKind = getBaseAccessKind(SGM, member,
37683809
SGFAccessKind::OwnedAddressRead,
37693810
strategy.getReadStrategy(),
3770-
baseFormalType);
3811+
baseFormalType,
3812+
/*for borrow*/ false);
37713813

37723814
// If they're the same kind, just use that.
37733815
if (readBaseKind == writeBaseKind)
@@ -3786,7 +3828,8 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
37863828
case AccessStrategy::DispatchToAccessor:
37873829
case AccessStrategy::DispatchToDistributedThunk: {
37883830
auto accessor = member->getOpaqueAccessor(strategy.getAccessor());
3789-
return getBaseAccessKindForAccessor(SGM, accessor, baseFormalType);
3831+
return getBaseAccessKindForAccessor(SGM, accessor, baseFormalType,
3832+
forBorrowExpr);
37903833
}
37913834
}
37923835
llvm_unreachable("bad access strategy");
@@ -3863,7 +3906,8 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
38633906

38643907
LValue lv = visitRec(e->getBase(),
38653908
getBaseAccessKind(SGF.SGM, var, accessKind, strategy,
3866-
getBaseFormalType(e->getBase())),
3909+
getBaseFormalType(e->getBase()),
3910+
/* for borrow */ false),
38673911
getBaseOptions(options, strategy));
38683912
assert(lv.isValid());
38693913

@@ -4069,7 +4113,8 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e,
40694113

40704114
LValue lv = visitRec(e->getBase(),
40714115
getBaseAccessKind(SGF.SGM, decl, accessKind, strategy,
4072-
getBaseFormalType(e->getBase())),
4116+
getBaseFormalType(e->getBase()),
4117+
/*for borrow*/ false),
40734118
getBaseOptions(options, strategy));
40744119
assert(lv.isValid());
40754120

@@ -4426,7 +4471,8 @@ LValue SILGenFunction::emitPropertyLValue(SILLocation loc, ManagedValue base,
44264471
F.getResilienceExpansion());
44274472

44284473
auto baseAccessKind =
4429-
getBaseAccessKind(SGM, ivar, accessKind, strategy, baseFormalType);
4474+
getBaseAccessKind(SGM, ivar, accessKind, strategy, baseFormalType,
4475+
/*for borrow*/ false);
44304476

44314477
LValueTypeData baseTypeData =
44324478
getValueTypeData(baseAccessKind, baseFormalType, base.getValue());
@@ -5149,7 +5195,8 @@ RValue SILGenFunction::emitRValueForStorageLoad(
51495195
if (!base) return LValue();
51505196

51515197
auto baseAccess = getBaseAccessKind(SGM, storage, accessKind,
5152-
strategy, baseFormalType);
5198+
strategy, baseFormalType,
5199+
/*for borrow*/ false);
51535200
return LValue::forValue(baseAccess, base, baseFormalType);
51545201
}();
51555202

‎test/IRGen/Inputs/large_c.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,14 @@ struct SamplesType {
2929
void* Z;
3030
void* AA;
3131
};
32+
33+
34+
typedef struct _ContainedType {
35+
unsigned int f1;
36+
float f2;
37+
} __attribute__((packed)) ContainedType;
38+
39+
typedef struct _ContainerType {
40+
char x1;
41+
ContainedType l[10];
42+
} __attribute__((packed)) ContainerType;

‎test/IRGen/loadable_by_address_reg2mem.sil

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ struct Y {
2929
var y2: X
3030
}
3131

32+
class C1 {
33+
}
34+
35+
sil_vtable C1 {
36+
}
37+
38+
struct Small {
39+
var x1 : Int
40+
}
41+
3242
// CHECK: sil @test1 : $@convention(thin) () -> () {
3343
// CHECK: bb0:
3444
// CHECK: %0 = alloc_stack $Optional<X>
@@ -229,3 +239,33 @@ bb0(%0 : $SamplesType):
229239
%t = tuple ()
230240
return %t : $()
231241
}
242+
243+
// In this test case Container type is identified as not large but contained
244+
// type is.
245+
sil @test9 : $@convention(thin) () -> () {
246+
bb0:
247+
%0 = alloc_stack $_ContainerType
248+
%1 = load %0 : $*_ContainerType
249+
%2 = alloc_stack $(_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType,
250+
_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType)
251+
252+
%3 = struct_extract %1 : $_ContainerType, #_ContainerType.l
253+
store %3 to %2 : $*(_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType,
254+
_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType)
255+
dealloc_stack %2 : $*(_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType,
256+
_ContainedType, _ContainedType, _ContainedType, _ContainedType, _ContainedType)
257+
dealloc_stack %0 : $*_ContainerType
258+
%t = tuple ()
259+
return %t : $()
260+
}
261+
262+
sil @test10 : $@convention(thin) (@in_guaranteed C1, @in Small) -> () {
263+
bb0(%0 : $*C1, %1 : $*Small):
264+
%2 = load %1 : $*Small
265+
%3 = load %0 : $*C1
266+
retain_value %3 : $C1
267+
store %2 to %1 : $*Small
268+
retain_value %3 : $C1
269+
%t = tuple ()
270+
return %t : $()
271+
}

0 commit comments

Comments
 (0)