Skip to content

Commit

Permalink
Merge pull request swiftlang#23526 from gottesmm/pr-8a32670da832a8c0a…
Browse files Browse the repository at this point in the history 
…68f4aaf94d9ff512d133fbd
  • Loading branch information
@swift-ci
swift-ci authored Mar 25, 2019
2 parents 68d5e37 + 4ebd6d0 commit a87988b
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 171 deletions.
148 changes: 14 additions & 134 deletions lib/SILGen/SILGenExpr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2030,150 +2030,30 @@ RValue RValueEmitter::visitTupleExpr(TupleExpr *E, SGFContext C) {
return result;
}

namespace {

/// A helper function with context that tries to emit member refs of nominal
/// types avoiding the conservative lvalue logic.
class NominalTypeMemberRefRValueEmitter {
using SelfTy = NominalTypeMemberRefRValueEmitter;

/// The member ref expression we are emitting.
MemberRefExpr *Expr;

/// The passed in SGFContext.
SGFContext Context;

/// The typedecl of the base expression of the member ref expression.
NominalTypeDecl *Base;

/// The field of the member.
VarDecl *Field;

public:

NominalTypeMemberRefRValueEmitter(MemberRefExpr *Expr, SGFContext Context,
NominalTypeDecl *Base)
: Expr(Expr), Context(Context), Base(Base),
Field(cast<VarDecl>(Expr->getMember().getDecl())) {}

/// Emit the RValue.
Optional<RValue> emit(SILGenFunction &SGF) {
// If we don't have a class or a struct, bail.
if (!isa<ClassDecl>(Base) && !isa<StructDecl>(Base))
return None;

// Check that we have a stored access strategy. If we don't bail.
AccessStrategy strategy =
Field->getAccessStrategy(Expr->getAccessSemantics(), AccessKind::Read,
SGF.SGM.M.getSwiftModule(),
SGF.F.getResilienceExpansion());
if (strategy.getKind() != AccessStrategy::Storage)
return None;

FormalEvaluationScope scope(SGF);
if (isa<StructDecl>(Base))
return emitStructDecl(SGF);
assert(isa<ClassDecl>(Base) && "Expected class");
return emitClassDecl(SGF);
}

NominalTypeMemberRefRValueEmitter(const SelfTy &) = delete;
NominalTypeMemberRefRValueEmitter(SelfTy &&) = delete;
~NominalTypeMemberRefRValueEmitter() = default;

private:
RValue emitStructDecl(SILGenFunction &SGF) {
ManagedValue base =
SGF.emitRValueAsSingleValue(Expr->getBase(),
SGFContext::AllowImmediatePlusZero);
CanType baseFormalType =
Expr->getBase()->getType()->getCanonicalType();
assert(baseFormalType->isMaterializable());

RValue result =
SGF.emitRValueForStorageLoad(Expr, base, baseFormalType,
Expr->isSuper(),
Field, {},
Expr->getMember().getSubstitutions(),
Expr->getAccessSemantics(),
Expr->getType(), Context);
return result;
}

Optional<RValue> emitClassDecl(SILGenFunction &SGF) {
// If guaranteed plus zero is not ok, we bail.
if (!Context.isGuaranteedPlusZeroOk())
return None;

// If the field is not a let, bail. We need to use the lvalue logic.
if (!Field->isLet())
return None;

// If we are emitting a delegating init super and we have begun the
// super.init call, since self has been exclusively borrowed, we need to be
// conservative and use the lvalue machinery. This ensures that we properly
// create FormalEvaluationScopes around the access to self.
//
// TODO: This currently turns off this optimization for /all/ classes that
// are accessed as a direct argument to a super.init call. In the future, we
// should be able to be less conservative here by pattern matching if
// something /can not/ be self.
if (SGF.SelfInitDelegationState == SILGenFunction::DidExclusiveBorrowSelf)
return None;

// Ok, now we know that we are able to emit our base at guaranteed plus zero
// emit base.
ManagedValue base =
SGF.emitRValueAsSingleValue(Expr->getBase(), Context);

CanType baseFormalType =
Expr->getBase()->getType()->getCanonicalType();
assert(baseFormalType->isMaterializable());

// And then emit our property using whether or not base is at +0 to
// discriminate whether or not the base was guaranteed.
RValue result =
SGF.emitRValueForStorageLoad(Expr, base, baseFormalType,
Expr->isSuper(),
Field, {},
Expr->getMember().getSubstitutions(),
Expr->getAccessSemantics(),
Expr->getType(), Context,
base.isPlusZeroRValueOrTrivial());
return std::move(result);
}
};

} // end anonymous namespace

RValue RValueEmitter::visitMemberRefExpr(MemberRefExpr *E, SGFContext C) {
assert(!E->getType()->is<LValueType>() &&
RValue RValueEmitter::visitMemberRefExpr(MemberRefExpr *e,
SGFContext resultCtx) {
assert(!e->getType()->is<LValueType>() &&
"RValueEmitter shouldn't be called on lvalues");

if (isa<TypeDecl>(E->getMember().getDecl())) {
if (isa<TypeDecl>(e->getMember().getDecl())) {
// Emit the metatype for the associated type.
visit(E->getBase());
SILValue MT =
SGF.B.createMetatype(E, SGF.getLoweredLoadableType(E->getType()));
return RValue(SGF, E, ManagedValue::forUnmanaged(MT));
visit(e->getBase());
SILValue mt =
SGF.B.createMetatype(e, SGF.getLoweredLoadableType(e->getType()));
return RValue(SGF, e, ManagedValue::forUnmanaged(mt));
}

// If we have a nominal type decl as our base, try to emit the base rvalue's
// member using special logic that will let us avoid extra retains
// and releases.
if (auto *N = E->getBase()->getType()->getNominalOrBoundGenericNominal())
if (auto RV = NominalTypeMemberRefRValueEmitter(E, C, N).emit(SGF))
return RValue(std::move(RV.getValue()));

// Everything else should use the l-value logic.

// Any writebacks for this access are tightly scoped.
FormalEvaluationScope scope(SGF);

LValue lv = SGF.emitLValue(E, SGFAccessKind::OwnedObjectRead);
// We can't load at +0 without further analysis, since the formal access into
// the lvalue will end immediately.
return SGF.emitLoadOfLValue(E, std::move(lv), C.withFollowingSideEffects());
LValue lv = SGF.emitLValue(e, SGFAccessKind::OwnedObjectRead);

// Otherwise, we can't load at +0 without further analysis, since the formal
// access into the lvalue will end immediately.
return SGF.emitLoadOfLValue(e, std::move(lv),
resultCtx.withFollowingSideEffects());
}

RValue RValueEmitter::visitDynamicMemberRefExpr(DynamicMemberRefExpr *E,
Expand Down
32 changes: 22 additions & 10 deletions lib/SILGen/SILGenLValue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,13 +721,16 @@ namespace {

ManagedValue project(SILGenFunction &SGF, SILLocation loc,
ManagedValue base) && override {
assert(base.getType().isObject() &&
"base for ref element component must be an object");
assert(base.getType().hasReferenceSemantics() &&
"base for ref element component must be a reference type");

// Borrow the ref element addr using formal access. If we need the ref
// element addr, we will load it in this expression.
base = base.formalAccessBorrow(SGF, loc);
if (base.getType().isAddress()) {
base = SGF.B.createFormalAccessLoadBorrow(loc, base);
} else {
base = base.formalAccessBorrow(SGF, loc);
}
SILValue result =
SGF.B.createRefElementAddr(loc, base.getUnmanagedValue(),
Field, SubstFieldType);
Expand Down Expand Up @@ -1903,11 +1906,19 @@ namespace {
RValue
TranslationPathComponent::get(SILGenFunction &SGF, SILLocation loc,
ManagedValue base, SGFContext c) && {
// Load the original value.
RValue baseVal(SGF, loc, getSubstFormalType(),
SGF.emitLoad(loc, base.getValue(),
SGF.getTypeLowering(base.getType()),
SGFContext(), IsNotTake));
// Inline constructor.
RValue baseVal = [&]() -> RValue {
// If our base is an object, just put it into an RValue and return.
if (base.getType().isObject()) {
return RValue(SGF, loc, getSubstFormalType(), base);
}

// Otherwise, load the value and put it into an RValue.
return RValue(SGF, loc, getSubstFormalType(),
SGF.emitLoad(loc, base.getValue(),
SGF.getTypeLowering(base.getType()),
SGFContext(), IsNotTake));
}();

// Map the base value to its substituted representation.
return std::move(*this).translate(SGF, loc, std::move(baseVal), c);
Expand All @@ -1916,6 +1927,8 @@ TranslationPathComponent::get(SILGenFunction &SGF, SILLocation loc,
void TranslationPathComponent::set(SILGenFunction &SGF, SILLocation loc,
ArgumentSource &&valueSource,
ManagedValue base) && {
assert(base.getType().isAddress() &&
"Only support setting bases that have addresses");
RValue value = std::move(valueSource).getAsRValue(SGF);

// Map the value to the original pattern.
Expand Down Expand Up @@ -2229,6 +2242,7 @@ static ManagedValue visitRecNonInOutBase(SILGenLValue &SGL, Expr *e,
}

// Ok, at this point we know that re-abstraction is not required.

SGFContext ctx;

if (auto *dre = dyn_cast<DeclRefExpr>(e)) {
Expand Down Expand Up @@ -4136,8 +4150,6 @@ void SILGenFunction::emitAssignToLValue(SILLocation loc, RValue &&src,
void SILGenFunction::emitAssignToLValue(SILLocation loc,
ArgumentSource &&src,
LValue &&dest) {
assert(dest.getAccessKind() == SGFAccessKind::Write);

// Enter a FormalEvaluationScope so that formal access to independent LValue
// components do not overlap. Furthermore, use an ArgumentScope to force
// cleanup of materialized LValues immediately, before evaluating the next
Expand Down
35 changes: 23 additions & 12 deletions test/SILGen/guaranteed_self.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,36 +451,45 @@ class LetFieldClass {
// CHECK-LABEL: sil hidden [ossa] @$s15guaranteed_self13LetFieldClassC10letkMethod{{[_0-9a-zA-Z]*}}F : $@convention(method) (@guaranteed LetFieldClass) -> () {
// CHECK: bb0([[CLS:%.*]] : @guaranteed $LetFieldClass):
// CHECK: [[KRAKEN_ADDR:%.*]] = ref_element_addr [[CLS]] : $LetFieldClass, #LetFieldClass.letk
// CHECK-NEXT: [[KRAKEN:%.*]] = load_borrow [[KRAKEN_ADDR]]
// CHECK-NEXT: [[KRAKEN:%.*]] = load [copy] [[KRAKEN_ADDR]]
// CHECK-NEXT: [[KRAKEN_METH:%.*]] = class_method [[KRAKEN]]
// CHECK-NEXT: apply [[KRAKEN_METH]]([[KRAKEN]])
// CHECK: [[KRAKEN_ADDR:%.*]] = ref_element_addr [[CLS]] : $LetFieldClass, #LetFieldClass.letk
// CHECK-NEXT: [[KRAKEN:%.*]] = load [copy] [[KRAKEN_ADDR]]
// CHECK: [[BORROWED_KRAKEN:%.*]] = begin_borrow [[KRAKEN]]
// CHECK: [[REBORROWED_KRAKEN:%.*]] = begin_borrow [[KRAKEN]]
// CHECK: [[DESTROY_SHIP_FUN:%.*]] = function_ref @$s15guaranteed_self11destroyShipyyAA6KrakenCF : $@convention(thin) (@guaranteed Kraken) -> ()
// CHECK-NEXT: apply [[DESTROY_SHIP_FUN]]([[BORROWED_KRAKEN]])
// CHECK-NEXT: end_borrow [[BORROWED_KRAKEN]]
// CHECK-NEXT: apply [[DESTROY_SHIP_FUN]]([[REBORROWED_KRAKEN]])
// CHECK-NEXT: end_borrow [[REBORROWED_KRAKEN]]
// CHECK-NEXT: destroy_value [[KRAKEN]]
// CHECK-NEXT: [[KRAKEN_BOX:%.*]] = alloc_box ${ var Kraken }
// CHECK-NEXT: [[PB:%.*]] = project_box [[KRAKEN_BOX]]
// CHECK-NEXT: [[KRAKEN_ADDR:%.*]] = ref_element_addr [[CLS]] : $LetFieldClass, #LetFieldClass.letk
// CHECK-NEXT: [[KRAKEN2:%.*]] = load [copy] [[KRAKEN_ADDR]]
// CHECK-NEXT: store [[KRAKEN2]] to [init] [[PB]]
// CHECK-NEXT: [[KRAKEN:%.*]] = load [copy] [[KRAKEN_ADDR]]
// CHECK-NEXT: store [[KRAKEN]] to [init] [[PB]]
// CHECK-NEXT: [[READ:%.*]] = begin_access [read] [unknown] [[PB]] : $*Kraken
// CHECK-NEXT: [[KRAKEN_COPY:%.*]] = load [copy] [[READ]]
// CHECK-NEXT: end_access [[READ]] : $*Kraken
// CHECK: [[DESTROY_SHIP_FUN:%.*]] = function_ref @$s15guaranteed_self11destroyShipyyAA6KrakenCF : $@convention(thin) (@guaranteed Kraken) -> ()
// CHECK-NEXT: apply [[DESTROY_SHIP_FUN]]([[KRAKEN_COPY]])
// CHECK-NEXT: destroy_value [[KRAKEN_COPY]]
// CHECK-NEXT: destroy_value [[KRAKEN_BOX]]
// CHECK-NEXT: destroy_value [[KRAKEN]]
// CHECK-NEXT: tuple
// CHECK-NEXT: return
// CHECK: } // end sil function
func letkMethod() {
letk.enrage()
let ll = letk
destroyShip(ll)
var lv = letk
destroyShip(lv)
do {
letk.enrage()
}

do {
let ll = letk
destroyShip(ll)
}

do {
var lv = letk
destroyShip(lv)
}
}

// CHECK-LABEL: sil hidden [ossa] @$s15guaranteed_self13LetFieldClassC10varkMethod{{[_0-9a-zA-Z]*}}F : $@convention(method) (@guaranteed LetFieldClass) -> () {
Expand Down Expand Up @@ -534,6 +543,8 @@ class ClassIntTreeNode {
// CHECK-NOT: destroy_value
// CHECK: copy_value
// CHECK-NOT: copy_value
// CHECK: destroy_value
// CHECK: destroy_value
// CHECK-NOT: destroy_value
// CHECK: return
func find(_ v : Int) -> ClassIntTreeNode {
Expand Down
26 changes: 15 additions & 11 deletions test/SILGen/let_decls.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,22 +407,26 @@ struct StructMemberTest {
}
// CHECK-LABEL: sil hidden [ossa] @$s9let_decls16StructMemberTestV016testRecursiveIntD4LoadSiyF : $@convention(method) (@guaranteed StructMemberTest)
// CHECK: bb0([[ARG:%.*]] : @guaranteed $StructMemberTest):
// CHECK: debug_value %0 : $StructMemberTest, let, name "self"
// CHECK: %2 = struct_extract %0 : $StructMemberTest, #StructMemberTest.s
// CHECK: %3 = struct_extract %2 : $AnotherStruct, #AnotherStruct.i
// CHECK-NOT: destroy_value %0 : $StructMemberTest
// CHECK: return %3 : $Int
// CHECK: [[EXT_1:%.*]] = struct_extract [[ARG]] : $StructMemberTest, #StructMemberTest.s
// CHECK: [[EXT_1_COPY:%.*]] = copy_value [[EXT_1]]
// CHECK: [[EXT_1_COPY_BORROW:%.*]] = begin_borrow [[EXT_1_COPY]]
// CHECK: [[EXT_2:%.*]] = struct_extract [[EXT_1_COPY_BORROW]] : $AnotherStruct, #AnotherStruct.i
// CHECK: destroy_value [[EXT_1_COPY]]
// CHECK: return [[EXT_2]] : $Int
// CHECK: } // end sil function '$s9let_decls16StructMemberTestV016testRecursiveIntD4LoadSiyF'

func testTupleMemberLoad() -> Int {
return t.1.i
}
// CHECK-LABEL: sil hidden [ossa] @$s9let_decls16StructMemberTestV09testTupleD4LoadSiyF : $@convention(method) (@guaranteed StructMemberTest)
// CHECK: bb0(%0 : @guaranteed $StructMemberTest):
// CHECK-NEXT: debug_value %0 : $StructMemberTest, let, name "self"
// CHECK-NEXT: [[T0:%.*]] = struct_extract %0 : $StructMemberTest, #StructMemberTest.t
// CHECK-NEXT: ({{%.*}}, [[T2:%.*]]) = destructure_tuple [[T0]]
// CHECK-NEXT: [[T3:%.*]] = struct_extract [[T2]] : $AnotherStruct, #AnotherStruct.i
// CHECK-NEXT: return [[T3]] : $Int
// CHECK: bb0([[ARG]] : @guaranteed $StructMemberTest):
// CHECK: [[T0:%.*]] = struct_extract [[ARG]] : $StructMemberTest, #StructMemberTest.t
// CHECK: [[T0_COPY:%.*]] = copy_value [[T0]]
// CHECK: ({{%.*}}, [[T2:%.*]]) = destructure_tuple [[T0_COPY]]
// CHECK: [[T2_BORROW:%.*]] = begin_borrow [[T2]]
// CHECK: [[T3:%.*]] = struct_extract [[T2_BORROW]] : $AnotherStruct, #AnotherStruct.i
// CHECK: return [[T3]] : $Int
// CHECK: } // end sil function '$s9let_decls16StructMemberTestV09testTupleD4LoadSiyF'

}

Expand Down
2 changes: 1 addition & 1 deletion test/SILGen/property_abstraction.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ struct Foo<T, U> {
// CHECK-LABEL: sil hidden [ossa] @$s20property_abstraction4getF{{[_0-9a-zA-Z]*}}Foo{{.*}}F : $@convention(thin) (@guaranteed Foo<Int, Int>) -> @owned @callee_guaranteed (Int) -> Int {
// CHECK: bb0([[X_ORIG:%.*]] : @guaranteed $Foo<Int, Int>):
// CHECK: [[F_ORIG:%.*]] = struct_extract [[X_ORIG]] : $Foo<Int, Int>, #Foo.f
// CHECK: [[F_ORIG_COPY:%.*]] = copy_value [[F_ORIG]]
// CHECK: [[REABSTRACT_FN:%.*]] = function_ref @$s{{.*}}TR :
// CHECK: [[F_ORIG_COPY:%.*]] = copy_value [[F_ORIG]]
// CHECK: [[F_SUBST:%.*]] = partial_apply [callee_guaranteed] [[REABSTRACT_FN]]([[F_ORIG_COPY]])
// CHECK: return [[F_SUBST]]
// CHECK: } // end sil function '$s20property_abstraction4getF{{[_0-9a-zA-Z]*}}F'
Expand Down
4 changes: 1 addition & 3 deletions test/SILOptimizer/access_marker_verify.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -904,11 +904,9 @@ internal struct CanCastStruct<Base : Hashable> : CanCast {
// CHECK: checked_cast_addr_br take_always CanCast in [[TEMP_BASE]] : $*CanCast to CanCastStruct<T> in [[TEMP_SUB_ADR]] : $*CanCastStruct<T>
// CHECK-NOT: begin_access
// CHECK: [[TEMP_DATA:%.*]] = unchecked_take_enum_data_addr [[TEMP_SUB]] : $*Optional<CanCastStruct<T>>, #Optional.some!enumelt.1
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unsafe] [[TEMP_DATA]] : $*CanCastStruct<T>
// CHECK: [[BASE_ADR:%.*]] = struct_element_addr [[ACCESS]] : $*CanCastStruct<T>, #CanCastStruct.base
// CHECK-NOT: begin_access
// CHECK: [[BASE_ADR:%.*]] = struct_element_addr [[TEMP_DATA]] : $*CanCastStruct<T>, #CanCastStruct.base
// CHECK: copy_addr [[BASE_ADR]] to [initialization] [[OUT_ENUM]] : $*T
// CHECK: end_access [[ACCESS]] : $*CanCastStruct<T>
// CHECK-NOT: begin_access
// CHECK: inject_enum_addr %0 : $*Optional<T>, #Optional.some!enumelt.1
// CHECK-LABEL: } // end sil function '$s20access_marker_verify13CanCastStructV5unboxqd__SgySHRd__lF'
Expand Down

0 comments on commit a87988b

Please sign in to comment.