Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
Could we use the method implementation pointer as the speculation key instead of the isa pointer? That makes the load chain a little longer, but should give you a guaranteed exhaustive key for Swift methods even with mixed ObjC heritage, since artificial ObjC subclasses still aren't allowed to override Swift methods that aren't explicitly `dynamic`.
-Joe
···
On Feb 2, 2016, at 9:54 AM, Arnold Schwaighofer via swift-dev <swift-dev@swift.org> wrote:
Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
Yes we could do something like this as an optimization I imagine. That should be done separately from this correctness bug, though.
···
On Feb 2, 2016, at 10:02 AM, Joe Groff <jgroff@apple.com> wrote:
Could we use the method implementation pointer as the speculation key instead of the isa pointer? That makes the load chain a little longer, but should give you a guaranteed exhaustive key for Swift methods even with mixed ObjC heritage, since artificial ObjC subclasses still aren't allowed to override Swift methods that aren't explicitly `dynamic`.
-Joe
On Feb 2, 2016, at 9:54 AM, Arnold Schwaighofer via swift-dev <swift-dev@swift.org> wrote:
Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
Could we use the method implementation pointer as the speculation key instead of the isa pointer? That makes the load chain a little longer, but should give you a guaranteed exhaustive key for Swift methods even with mixed ObjC heritage, since artificial ObjC subclasses still aren't allowed to override Swift methods that aren't explicitly `dynamic`.
I filed a radar about using the method implementation pointer as the speculation key instead of the isa pointer. See rdar://18508812 for more details.
One of the reasons why it was not implemented yet is the fact that it makes jump threading of checked_cast_br almost impossible, because because each speculatively devirtualized class_method would use a different key.
-Roman
···
On 02 Feb 2016, at 10:02, Joe Groff via swift-dev <swift-dev@swift.org> wrote:
-Joe
On Feb 2, 2016, at 9:54 AM, Arnold Schwaighofer via swift-dev <swift-dev@swift.org> wrote:
Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
Yes we could do something like this as an optimization I imagine. That should be done separately from this correctness bug, though.
Agreed. The change LGTM as is.
-Joe
···
On Feb 2, 2016, at 10:06 AM, Arnold Schwaighofer <aschwaighofer@apple.com> wrote:
On Feb 2, 2016, at 10:02 AM, Joe Groff <jgroff@apple.com> wrote:
Could we use the method implementation pointer as the speculation key instead of the isa pointer? That makes the load chain a little longer, but should give you a guaranteed exhaustive key for Swift methods even with mixed ObjC heritage, since artificial ObjC subclasses still aren't allowed to override Swift methods that aren't explicitly `dynamic`.
-Joe
On Feb 2, 2016, at 9:54 AM, Arnold Schwaighofer via swift-dev <swift-dev@swift.org> wrote:
Perform a dynamic method call if a class has objc ancestry in specula
tive devirt as fallback.
If a class has an @objc ancestry this class can be dynamically overridden and
therefore we don't know the default case even if we see the full class
hierarchy.
rdar://23228386
Explanation:
Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
private class A : NSObject {
func foo() {...}
}
private class B : A {
override foo() {...}
}
Before:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else {
B.foo(a)
}
}
After:
callAnA(a : A) {
if (a isa A) {
A.foo(a)
} else if (a isa B) {
B.foo(a)
} else a.foo(a) // call through class method table.
}
Scope:
The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
Testing:
There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
Reviewed by:
Roman, the author of the speculative virtualization pass, and Slava also took a look at it.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev