Skip to content

IRGen: Don't hoist metadata for weakly linked types #21223

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 2 commits into from
Dec 12, 2018
Merged

IRGen: Don't hoist metadata for weakly linked types #21223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f34bdc
IRGen: Don't hoist metadata for weakly linked types
aschwaighofer Dec 11, 2018
0c01906
Address review feedback
aschwaighofer Dec 12, 2018
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
12 changes: 12 additions & 0 deletions lib/IRGen/AllocStackHoisting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ static bool isHoistable(AllocStackInst *Inst, irgen::IRGenModule &Mod) {
if (TI.isFixedSize())
return false;

// Don't hoist weakly imported (weakly linked) types.
bool foundWeaklyImported =
SILTy.getASTType().findIf([&Mod](CanType type) -> bool {
if (auto nominal = type->getNominalOrBoundGenericNominal())
if (nominal->isWeakImported(Mod.getSILModule().getSwiftModule())) {
return true;
}
return false;
});
if (foundWeaklyImported)
return false;

// Don't hoist generics with opened archetypes. We would have to hoist the
// open archetype instruction which might not be possible.
return Inst->getTypeDependentOperands().empty();
Expand Down
12 changes: 12 additions & 0 deletions test/IRGen/Inputs/weak_import_native_helper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,15 @@ extension ProtocolWithWeakMembers {

public protocol BaseP {}
@_weakLinked extension S : BaseP {}


public func getVersion() -> Int {
// Used to return 0.
return 1
}

@_weakLinked public struct ResilientStruct {
public init() {}

public func fn(_ x: Int) {}
}
165 changes: 110 additions & 55 deletions test/IRGen/weak_import_native.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,100 +5,100 @@

import weak_import_native_helper

// CHECK-DAG: @"$s25weak_import_native_helper23ProtocolWithWeakMembersP1TAC_AA05OtherE0Tn" = extern_weak global %swift.protocol_requirement
// CHECK-DAG: @"$s1T25weak_import_native_helper23ProtocolWithWeakMembersPTl" = extern_weak global %swift.protocol_requirement
// CHECK-DAG: @"$s25weak_import_native_helper23ProtocolWithWeakMembersP1fyyFTq" = extern_weak global %swift.method_descriptor
// CHECK-DAG: declare extern_weak swiftcc void @"$s25weak_import_native_helper23ProtocolWithWeakMembersPAAE1fyyF"(%swift.type*, i8**, %swift.opaque* noalias nocapture swiftself)
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper23ProtocolWithWeakMembersP1TAC_AA05OtherE0Tn" = extern_weak global %swift.protocol_requirement
// CHECK-DAG-LABEL: @"$s1T25weak_import_native_helper23ProtocolWithWeakMembersPTl" = extern_weak global %swift.protocol_requirement
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper23ProtocolWithWeakMembersP1fyyFTq" = extern_weak global %swift.method_descriptor
// CHECK-DAG-LABEL: declare extern_weak swiftcc void @"$s25weak_import_native_helper23ProtocolWithWeakMembersPAAE1fyyF"(%swift.type*, i8**, %swift.opaque* noalias nocapture swiftself)
struct ConformsToProtocolWithWeakMembers : ProtocolWithWeakMembers {}

func testTopLevel() {
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper2fnyyF"()
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper2fnyyF"()
fn()

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper12globalStoredSivg"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper12globalStoredSivs"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper12globalStoredSivg"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper12globalStoredSivs"
let x = globalStored
globalStored = x
globalStored += 1

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper14globalComputedSivg"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper14globalComputedSivs"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper14globalComputedSivg"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper14globalComputedSivs"
let y = globalComputed
globalComputed = y
globalComputed += 1
}

func testStruct() {
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVACycfC"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVACycfC"
var s = S()

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV2fnyyF"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV2fnyyF"
s.fn()

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivg"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivs"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivM"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivg"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivs"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV10storedPropSivM"
let x = s.storedProp
s.storedProp = x
s.storedProp += 1

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivg"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivs"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivM"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivg"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivs"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SV12computedPropSivM"
let y = s.computedProp
s.computedProp = y
s.computedProp += 1

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2icig"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2icis"
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2iciM"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2icig"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2icis"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1SVyS2iciM"
let z = s[0]
s[0] = z
s[0] += 1

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper5WeakSV0A6MemberyyF"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper5WeakSV0A6MemberyyF"
let w = WeakS()
w.weakMember()
}

func testEnum() {
// CHECK-DAG: @"$s25weak_import_native_helper1EO6strongyA2CmFWC" = external constant i32
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper1EO6strongyA2CmFWC" = external constant i32
_ = E.strong

// CHECK-DAG: @"$s25weak_import_native_helper1EO0A0yA2CmFWC" = extern_weak constant i32
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper1EO0A0yA2CmFWC" = extern_weak constant i32
_ = E.weak

// CHECK-DAG: @"$s25weak_import_native_helper1EO11strongAssocyACSicACmFWC" = external constant i32
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper1EO11strongAssocyACSicACmFWC" = external constant i32
_ = E.strongAssoc(0)

// CHECK-DAG: @"$s25weak_import_native_helper1EO0A5AssocyACSicACmFWC" = extern_weak constant i32
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper1EO0A5AssocyACSicACmFWC" = extern_weak constant i32
_ = E.weakAssoc(0)
}

func testClass() {
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1CCACycfC"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1CCACycfC"
let c = C()

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1CC2fnyyFTj"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1CC2fnyyFTj"
c.fn()

// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivgTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivsTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivMTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivgTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivsTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC10storedPropSivMTj"
let x = c.storedProp
c.storedProp = x
c.storedProp += 1

// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivgTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivsTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivMTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivgTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivsTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CC12computedPropSivMTj"
let y = c.computedProp
c.computedProp = y
c.computedProp += 1

// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2icigTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2icisTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2iciMTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2icigTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2icisTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1CCyS2iciMTj"
let z = c[0]
c[0] = z
c[0] += 1
Expand All @@ -108,33 +108,33 @@ class Sub : C {
deinit {
// This is correctly a strong symbol reference; the class is not declared
// weak.
// CHECK-DAG: declare swiftcc {{.+}} @"$s25weak_import_native_helper1CCfd"
// CHECK-DAG-LABEL: declare swiftcc {{.+}} @"$s25weak_import_native_helper1CCfd"
}
}

func testProtocolExistential(_ p: P) {
var mutP = p

// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1PP2fnyyFTj"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1PP2fnyyFTj"
p.fn()

// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivgTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivsTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivMTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivgTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivsTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PP4propSivMTj"
let x = p.prop
mutP.prop = x
mutP.prop += 1

// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2icigTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2icisTj"
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2iciMTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2icigTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2icisTj"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper1PPyS2iciMTj"
let z = p[0]
mutP[0] = z
mutP[0] += 1
}

func testProtocolGeneric<Impl: P>(_ type: Impl.Type) {
// CHECK-DAG: declare extern_weak {{.+}} @"$s25weak_import_native_helper1PPxycfCTj"
// CHECK-DAG-LABEL: declare extern_weak {{.+}} @"$s25weak_import_native_helper1PPxycfCTj"
var mutP = type.init()

mutP.fn()
Expand All @@ -149,29 +149,84 @@ func testProtocolGeneric<Impl: P>(_ type: Impl.Type) {
}

func testWeakTypes() -> [Any.Type] {
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakSVMa"
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakEOMa"
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakCCMa"
// CHECK-DAG: @"$s25weak_import_native_helper5WeakPMp" = extern_weak global %swift.protocol
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericSVMa"
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericEOMa"
// CHECK-DAG: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericCCMa"
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakSVMa"
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakEOMa"
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper5WeakCCMa"
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper5WeakPMp" = extern_weak global %swift.protocol
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericSVMa"
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericEOMa"
// CHECK-DAG-LABEL: declare extern_weak swiftcc %swift.metadata_response @"$s25weak_import_native_helper8GenericCCMa"
return [WeakS.self, WeakE.self, WeakC.self, WeakP.self, GenericS<Int>.self, GenericE<Int>.self, GenericC<Int>.self]
}

class WeakSub: WeakC {
deinit {
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper5WeakCCfd"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper5WeakCCfd"
}
}

class WeakGenericSub: GenericC<Int> {
deinit {
// CHECK-DAG: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper8GenericCCfd"
// CHECK-DAG-LABEL: declare extern_weak swiftcc {{.+}} @"$s25weak_import_native_helper8GenericCCfd"
}
}

protocol RefinesP : BaseP {}

// CHECK-DAG: @"$s25weak_import_native_helper1SVAA5BasePAAWP" = extern_weak global i8*
// CHECK-DAG-LABEL: @"$s25weak_import_native_helper1SVAA5BasePAAWP" = extern_weak global i8*
extension S : RefinesP {}

// We should not hoist the metadata accessor accross the version check.

// CHECK-DAG-LABEL: define{{.*}} void @"$s18weak_import_native28test_not_hoist_weakly_linkedyyF"()
// CHECK-NOT: 15ResilientStructVMa
// CHECK: getVersion
// CHECK: br
// CHECK: 15ResilientStructVMa
// CHECK: ret

public func test_not_hoist_weakly_linked() {
if getVersion() == 1 {
var _ = ResilientStruct()
}
}

// CHECK-DAG-LABEL: define{{.*}} void @"$s18weak_import_native29test_not_hoist_weakly_linked2yyF"()
// CHECK-NOT: 15ResilientStructVMa
// CHECK: getVersion
// CHECK: br
// CHECK: 15ResilientStructVMa
// CHECK: ret
public func test_not_hoist_weakly_linked2() {
if getVersion() == 1 {
var _ = (ResilientStruct(), 1)
}
}

struct One<T> {
var elt : T?
}

// CHECK-DAG-LABEL: define{{.*}} void @"$s18weak_import_native29test_not_hoist_weakly_linked3yyF"()
// CHECK-NOT: 15ResilientStructVMa
// CHECK: getVersion
// CHECK: br
// CHECK: 15ResilientStructVMa
// CHECK: ret
public func test_not_hoist_weakly_linked3() {
if getVersion() == 1 {
var _ = One(elt:ResilientStruct())
}
}

// CHECK-DAG-LABEL: define{{.*}} void @"$s18weak_import_native29test_not_hoist_weakly_linked4yyF"()
// CHECK-NOT: 15ResilientStructVMa
// CHECK: getVersion
// CHECK: br
// CHECK: 15ResilientStructVMa
// CHECK: ret
public func test_not_hoist_weakly_linked4() {
if getVersion() == 1 {
var _ = One(elt:(ResilientStruct(), 1))
}
}
28 changes: 9 additions & 19 deletions validation-test/Evolution/test_backward_deploy_protocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,8 @@ struct ConformsToOldWithNonDefault : OldProtocol {

BackwardDeployProtocolTest.test("OldProtocol") {
if getVersion() == 1 {
// FIXME: IRGen inserts the metadata load outside the version check
// apparently. Work around that here. <rdar://problem/46438608>
@inline(never) func helper() {
_ = ConformsToOldWithDefault().newMethod()
_ = ConformsToOldWithNonDefault().newMethod()
}

helper()
_ = ConformsToOldWithDefault().newMethod()
_ = ConformsToOldWithNonDefault().newMethod()
}
}

Expand Down Expand Up @@ -76,17 +70,13 @@ BackwardDeployProtocolTest.test("RefinedProtocol") {
// Witness tables that are weak-linked for various reasons
BackwardDeployProtocolTest.test("WeakWitnessTables") {
if getVersion() == 1 {
// FIXME: IRGen inserts the metadata load outside the version check
// apparently. Work around that here. <rdar://problem/46438608>
@inline(never) func helper() {
func f1<T : OtherProtocol>(_: T) {}
func f2<T : NewProtocol>(_: T) {}
func f3<T : NewConformanceProtocol>(_: T) {}

f1(OtherConforms())
f2(NewConforms())
f3(NewConformanceConforms())
}
func f1<T : OtherProtocol>(_: T) {}
func f2<T : NewProtocol>(_: T) {}
func f3<T : NewConformanceProtocol>(_: T) {}

f1(OtherConforms())
f2(NewConforms())
f3(NewConformanceConforms())
}
}

Expand Down
26 changes: 10 additions & 16 deletions validation-test/Evolution/test_backward_deploy_struct.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,19 @@ var BackwardDeployStructTest = TestSuite("BackwardDeployStruct")

BackwardDeployStructTest.test("ResilientStruct") {
if getVersion() == 1 {
// FIXME: IRGen inserts the metadata load outside the version check
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a second spot where I had this workaround in this directory, can you grep for it (sorry on my phone right now)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// apparently. Work around that here. <rdar://problem/46438608>
@inline(never) func helper() {
var s = ResilientStruct()
var s = ResilientStruct()

s.fn(s.storedProp)
s.storedProp = 1
s.storedProp += 1

s.fn(s.computedProp)
s.computedProp = 1
s.computedProp += 1
s.fn(s.storedProp)
s.storedProp = 1
s.storedProp += 1

s.fn(s[0])
s[0] = 1
s[0] += 1
}
s.fn(s.computedProp)
s.computedProp = 1
s.computedProp += 1

helper()
s.fn(s[0])
s[0] = 1
s[0] += 1
}
}

Expand Down