Skip to content

Fix for nested LoadExprs fix and cleanups #9625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 15, 2017
Merged

Fix for nested LoadExprs fix and cleanups #9625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b25945
SILGen: Fix overlapping LoadExprs
slavapestov May 13, 2017
8ba21d8
SILGen: Fix some accesses of values after they've been consumed
slavapestov May 13, 2017
159807d
SILGen: Add an assertion to OpenOpaqueExistentialComponent
slavapestov May 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/SILGen/SILGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,13 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction

/// Mapping from active opaque value expressions to their values,
/// along with a bit for each indicating whether it has been consumed yet.
llvm::DenseMap<OpaqueValueExpr *, OpaqueValueState> OpaqueValues;
llvm::SmallDenseMap<OpaqueValueExpr *, OpaqueValueState>
OpaqueValues;

/// A mapping from opaque value expressions to the open-existential
/// expression that determines them, used while lowering lvalues.
llvm::SmallDenseMap<OpaqueValueExpr *, OpenExistentialExpr *>
OpaqueValueExprs;

/// RAII object that introduces a temporary binding for an opaque value.
///
Expand Down
32 changes: 17 additions & 15 deletions lib/SILGen/SILGenLValue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,6 @@ SILGenFunction::getUnknownEnforcement(VarDecl *var) {
class LLVM_LIBRARY_VISIBILITY SILGenLValue
: public Lowering::ExprVisitor<SILGenLValue, LValue, AccessKind>
{
/// A mapping from opaque value expressions to the open-existential
/// expression that determines them.
llvm::SmallDenseMap<OpaqueValueExpr *, OpenExistentialExpr *>
openedExistentials;

public:
SILGenFunction &SGF;
Expand Down Expand Up @@ -591,6 +587,8 @@ namespace {
"base for open existential component must be an existential");
assert(base.getType().isAddress() &&
"base value of open-existential component was not an address?");
assert(base.getType().getPreferredExistentialRepresentation(SGF.SGM.M)
== ExistentialRepresentation::Opaque);

SILValue addr = SGF.B.createOpenExistentialAddr(
loc, base.getValue(), getTypeOfRValue().getAddressType(),
Expand Down Expand Up @@ -1909,11 +1907,11 @@ LValue SILGenLValue::visitDeclRefExpr(DeclRefExpr *e, AccessKind accessKind) {
LValue SILGenLValue::visitOpaqueValueExpr(OpaqueValueExpr *e,
AccessKind accessKind) {
// Handle an opaque lvalue that refers to an opened existential.
auto known = openedExistentials.find(e);
if (known != openedExistentials.end()) {
auto known = SGF.OpaqueValueExprs.find(e);
if (known != SGF.OpaqueValueExprs.end()) {
// Dig the open-existential expression out of the list.
OpenExistentialExpr *opened = known->second;
openedExistentials.erase(known);
SGF.OpaqueValueExprs.erase(known);

// Do formal evaluation of the underlying existential lvalue.
LValue lv = visitRec(opened->getExistentialValue(), accessKind);
Expand Down Expand Up @@ -2206,15 +2204,15 @@ LValue SILGenLValue::visitOpenExistentialExpr(OpenExistentialExpr *e,

// Record the fact that we're opening this existential. The actual
// opening operation will occur when we see the OpaqueValueExpr.
bool inserted = openedExistentials.insert({e->getOpaqueValue(), e}).second;
bool inserted = SGF.OpaqueValueExprs.insert({e->getOpaqueValue(), e}).second;
(void)inserted;
assert(inserted && "already have this opened existential?");

// Visit the subexpression.
LValue lv = visitRec(e->getSubExpr(), accessKind);

// Sanity check that we did see the OpaqueValueExpr.
assert(openedExistentials.count(e->getOpaqueValue()) == 0 &&
assert(SGF.OpaqueValueExprs.count(e->getOpaqueValue()) == 0 &&
"opened existential not removed?");
return lv;
}
Expand Down Expand Up @@ -2731,6 +2729,7 @@ static ManagedValue drillIntoComponent(SILGenFunction &SGF,
ManagedValue base,
AccessKind accessKind,
TSanKind tsanKind) {
bool isRValue = component.isRValue();
ManagedValue addr;
if (component.isPhysical()) {
addr = std::move(component.asPhysical()).offset(SGF, loc, base, accessKind);
Expand All @@ -2741,7 +2740,7 @@ static ManagedValue drillIntoComponent(SILGenFunction &SGF,

if (!SGF.getASTContext().LangOpts.DisableTsanInoutInstrumentation &&
SGF.getModule().getOptions().Sanitize == SanitizerKind::Thread &&
tsanKind == TSanKind::InoutAccess && !component.isRValue()) {
tsanKind == TSanKind::InoutAccess && !isRValue) {
emitTsanInoutAccess(SGF, loc, addr);
}

Expand Down Expand Up @@ -2781,6 +2780,9 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
// Any writebacks should be scoped to after the load.
FormalEvaluationScope scope(*this);

auto substFormalType = src.getSubstFormalType();
auto &rvalueTL = getTypeLowering(src.getTypeOfRValue());

ManagedValue addr;
PathComponent &&component =
drillToLastComponent(*this, loc, std::move(src), addr, AccessKind::Read);
Expand All @@ -2789,9 +2791,9 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
if (component.isPhysical()) {
addr = std::move(component.asPhysical())
.offset(*this, loc, addr, AccessKind::Read);
return RValue(*this, loc, src.getSubstFormalType(),
return RValue(*this, loc, substFormalType,
emitLoad(loc, addr.getValue(),
getTypeLowering(src.getTypeOfRValue()), C, IsNotTake,
rvalueTL, C, IsNotTake,
isGuaranteedValid));
}

Expand Down Expand Up @@ -2890,6 +2892,8 @@ void SILGenFunction::emitAssignLValueToLValue(SILLocation loc, LValue &&src,
return;
}

auto &rvalueTL = getTypeLowering(src.getTypeOfRValue());

auto srcAddr = emitAddressOfLValue(loc, std::move(src), AccessKind::Read)
.getUnmanagedValue();
auto destAddr = emitAddressOfLValue(loc, std::move(dest), AccessKind::Write)
Expand All @@ -2899,9 +2903,7 @@ void SILGenFunction::emitAssignLValueToLValue(SILLocation loc, LValue &&src,
B.createCopyAddr(loc, srcAddr, destAddr, IsNotTake, IsNotInitialization);
} else {
// If there's a semantic conversion necessary, do a load then assign.
auto loaded = emitLoad(loc, srcAddr, getTypeLowering(src.getTypeOfRValue()),
SGFContext(),
IsNotTake);
auto loaded = emitLoad(loc, srcAddr, rvalueTL, SGFContext(), IsNotTake);
loaded.assignInto(*this, loc, destAddr);
}
}
45 changes: 45 additions & 0 deletions test/SILGen/properties.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,3 +1111,48 @@ public class DerivedClassWithPublicProperty : BaseClassWithInternalProperty {
// CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]]([[SUPER]]) : $@convention(method) (@guaranteed BaseClassWithInternalProperty) -> ()
// CHECK-NEXT: destroy_value [[SUPER]] : $BaseClassWithInternalProperty
// CHECK: } // end sil function '_T010properties30DerivedClassWithPublicPropertyC1xytfg'

// Make sure that we can handle this AST:
// (load_expr
// (open_existential_expr
// (opaque_expr A)
// ...
// (load_expr
// (opaque_expr ))))

class ReferenceType {
var p: NonmutatingProtocol
init(p: NonmutatingProtocol) { self.p = p }
}

protocol NonmutatingProtocol {
var x: Int { get nonmutating set }
}

// sil hidden @_T010properties19overlappingLoadExpryAA13ReferenceTypeCz1c_tF : $@convention(thin) (@inout ReferenceType) -> () {
// CHECK: [[RESULT:%.*]] = alloc_stack $Int
// CHECK-NEXT: [[UNINIT:%.*]] = mark_uninitialized [var] [[RESULT]] : $*Int
// CHECK-NEXT: [[C_INOUT:%.*]] = begin_access [read] [unknown] %0 : $*ReferenceType
// CHECK-NEXT: [[C:%.*]] = load [copy] [[C_INOUT:%.*]] : $*ReferenceType
// CHECK-NEXT: end_access [[C_INOUT]] : $*ReferenceType
// CHECK-NEXT: [[C_FIELD_BOX:%.*]] = alloc_stack $NonmutatingProtocol
// CHECK-NEXT: [[GETTER:%.*]] = class_method [[C]] : $ReferenceType, #ReferenceType.p!getter.1 : (ReferenceType) -> () -> NonmutatingProtocol, $@convention(method) (@guaranteed ReferenceType) -> @out NonmutatingProtocol
// CHECK-NEXT: apply [[GETTER]]([[C_FIELD_BOX]], [[C]]) : $@convention(method) (@guaranteed ReferenceType) -> @out NonmutatingProtocol
// CHECK-NEXT: destroy_value [[C]] : $ReferenceType
// CHECK-NEXT: [[C_FIELD_PAYLOAD:%.*]] = open_existential_addr immutable_access [[C_FIELD_BOX]] : $*NonmutatingProtocol to $*@opened("{{.*}}") NonmutatingProtocol
// CHECK-NEXT: [[C_FIELD_COPY:%.*]] = alloc_stack $@opened("{{.*}}") NonmutatingProtocol
// CHECK-NEXT: copy_addr [[C_FIELD_PAYLOAD]] to [initialization] [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
// CHECK-NEXT: destroy_addr [[C_FIELD_BOX]] : $*NonmutatingProtocol
// CHECK-NEXT: [[GETTER:%.*]] = witness_method $@opened("{{.*}}") NonmutatingProtocol, #NonmutatingProtocol.x!getter.1 : <Self where Self : NonmutatingProtocol> (Self) -> () -> Int, %11 : $*@opened("{{.*}}") NonmutatingProtocol : $@convention(witness_method) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
// CHECK-NEXT: [[RESULT_VALUE:%.*]] = apply [[GETTER]]<@opened("{{.*}}") NonmutatingProtocol>([[C_FIELD_COPY]]) : $@convention(witness_method) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
// CHECK-NEXT: destroy_addr [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
// CHECK-NEXT: assign [[RESULT_VALUE]] to [[UNINIT]] : $*Int
// CHECK-NEXT: dealloc_stack [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
// CHECK-NEXT: dealloc_stack [[C_FIELD_BOX]] : $*NonmutatingProtocol
// CHECK-NEXT: dealloc_stack [[RESULT]] : $*Int
// CHECK-NEXT: tuple ()
// CHECK-NEXT: return

func overlappingLoadExpr(c: inout ReferenceType) {
_ = c.p.x
}