-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Split off objc_method / objc_super_method from class_method / super_method #12260
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.
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
Conversation
e33ccdc
to
a2e6772
Compare
…per_method This replaces the '[volatile]' flag. Now, class_method and super_method are only used for vtable dispatch. The witness_method instruction is still overloaded for use with both ObjC protocol requirements and Swift protocol requirements; the next step is to make it only mean the latter, also using objc_method for ObjC protocol calls.
Let's not use string literals as booleans here.
a2e6772
to
1632b73
Compare
@swift-ci Please test |
@swift-ci Please test source compatibility |
@swift-ci Please benchmark |
Build failed |
@swift-ci Please test |
@swift-ci Please test source compatibility |
@swift-ci Please benchmark |
Build comment file:Optimized (O)Regression (1)
No Changes (328)
Unoptimized (Onone)Regression (1)
No Changes (328)
Hardware Overview
|
This broke a number of bots, such as https://ci.swift.org/job/oss-swift_tools-R_stdlib-RD_test-simulator/324/. I'm not sure how this wasn't caught by "please test" but there's definitely a problem here. |
Oops, hang on, sorry. Not this PR. |
This looks like good progress, thanks @slavapestov! Ultimately I think we should also go as far as making the instructions just take a selector (in whatever form the |
I haven't looked, but just in case... does this require any updates to SIL.rst?
Michael
… On Oct 3, 2017, at 8:30 PM, Slava Pestov ***@***.***> wrote:
This is the first part of a refactoring to make Objective-C dispatch explicit in SIL, instead of relying on a poorly-named 'volatile' bit in the class_method / super_method / witness_method instructions.
The next step is to switch Objective-C protocol method calls over to use objc_method, since witness_method models Objective-C dispatch poorly.
The reason I'm doing this is because it is a general cleanup we've talked about for a while, and also because we might switch to a resilient method dispatch strategy where it would be advantageous to change class_method to take an isa pointer, and replace super_method with a super_metatype + class_method sequence. Since this separation doesn't make sense for Objective-C, I'm going to land this refactoring first.
Also, it might be possible to eliminate the dynamic_method instruction entirely now, but I haven't looked into that yet.
Unfortunately the new instructions do introduce a lot of new code, but some upcoming changes I'm planning on making to SILGen will eliminate the duplication for lowering call expressions a fair bit.
You can view, comment on, or merge this pull request online at:
#12260 <#12260>
Commit Summary
SILGen: Clean up class_method instruction construction a bit
SIL: Split off objc_method / objc_super_method from class_method / super_method
File Changes
M include/swift/SIL/PatternMatch.h <https://github.com/apple/swift/pull/12260/files#diff-0> (2)
M include/swift/SIL/SILBuilder.h <https://github.com/apple/swift/pull/12260/files#diff-1> (41)
M include/swift/SIL/SILCloner.h <https://github.com/apple/swift/pull/12260/files#diff-2> (31)
M include/swift/SIL/SILInstruction.h <https://github.com/apple/swift/pull/12260/files#diff-3> (57)
M include/swift/SIL/SILNodes.def <https://github.com/apple/swift/pull/12260/files#diff-4> (4)
M include/swift/Serialization/ModuleFormat.h <https://github.com/apple/swift/pull/12260/files#diff-5> (2)
M lib/IRGen/IRGenSIL.cpp <https://github.com/apple/swift/pull/12260/files#diff-6> (36)
M lib/IRGen/LoadableByAddress.cpp <https://github.com/apple/swift/pull/12260/files#diff-7> (13)
M lib/ParseSIL/ParseSIL.cpp <https://github.com/apple/swift/pull/12260/files#diff-8> (20)
M lib/SIL/SILInstruction.cpp <https://github.com/apple/swift/pull/12260/files#diff-9> (21)
M lib/SIL/SILInstructions.cpp <https://github.com/apple/swift/pull/12260/files#diff-10> (5)
M lib/SIL/SILOwnershipVerifier.cpp <https://github.com/apple/swift/pull/12260/files#diff-11> (2)
M lib/SIL/SILPrinter.cpp <https://github.com/apple/swift/pull/12260/files#diff-12> (15)
M lib/SIL/SILVerifier.cpp <https://github.com/apple/swift/pull/12260/files#diff-13> (94)
M lib/SIL/ValueOwnershipKindClassifier.cpp <https://github.com/apple/swift/pull/12260/files#diff-14> (2)
M lib/SILGen/SILGen.h <https://github.com/apple/swift/pull/12260/files#diff-15> (2)
M lib/SILGen/SILGenApply.cpp <https://github.com/apple/swift/pull/12260/files#diff-16> (43)
M lib/SILGen/SILGenBridging.cpp <https://github.com/apple/swift/pull/12260/files#diff-17> (6)
M lib/SILGen/SILGenBuilder.cpp <https://github.com/apple/swift/pull/12260/files#diff-18> (14)
M lib/SILGen/SILGenBuilder.h <https://github.com/apple/swift/pull/12260/files#diff-19> (7)
M lib/SILGen/SILGenFunction.cpp <https://github.com/apple/swift/pull/12260/files#diff-20> (7)
M lib/SILGen/SILGenFunction.h <https://github.com/apple/swift/pull/12260/files#diff-21> (2)
M lib/SILGen/SILGenPoly.cpp <https://github.com/apple/swift/pull/12260/files#diff-22> (22)
M lib/SILGen/SILGenThunk.cpp <https://github.com/apple/swift/pull/12260/files#diff-23> (16)
M lib/SILGen/SILGenType.cpp <https://github.com/apple/swift/pull/12260/files#diff-24> (2)
M lib/SILOptimizer/Analysis/BasicCalleeAnalysis.cpp <https://github.com/apple/swift/pull/12260/files#diff-25> (5)
M lib/SILOptimizer/Analysis/EscapeAnalysis.cpp <https://github.com/apple/swift/pull/12260/files#diff-26> (2)
M lib/SILOptimizer/LoopTransforms/LoopRotate.cpp <https://github.com/apple/swift/pull/12260/files#diff-27> (4)
M lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp <https://github.com/apple/swift/pull/12260/files#diff-28> (26)
M lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp <https://github.com/apple/swift/pull/12260/files#diff-29> (26)
M lib/SILOptimizer/Transforms/CSE.cpp <https://github.com/apple/swift/pull/12260/files#diff-30> (11)
M lib/SILOptimizer/Transforms/Outliner.cpp <https://github.com/apple/swift/pull/12260/files#diff-31> (84)
M lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp <https://github.com/apple/swift/pull/12260/files#diff-32> (5)
M lib/SILOptimizer/Utils/Devirtualize.cpp <https://github.com/apple/swift/pull/12260/files#diff-33> (5)
M lib/SILOptimizer/Utils/SILInliner.cpp <https://github.com/apple/swift/pull/12260/files#diff-34> (2)
M lib/Serialization/DeserializeSIL.cpp <https://github.com/apple/swift/pull/12260/files#diff-35> (24)
M lib/Serialization/SerializeSIL.cpp <https://github.com/apple/swift/pull/12260/files#diff-36> (53)
M test/IRGen/objc_alloc.sil <https://github.com/apple/swift/pull/12260/files#diff-37> (2)
M test/IRGen/partial_apply_objc.sil <https://github.com/apple/swift/pull/12260/files#diff-38> (8)
M test/SIL/Parser/overloaded_member.sil <https://github.com/apple/swift/pull/12260/files#diff-39> (4)
M test/SILGen/cf.swift <https://github.com/apple/swift/pull/12260/files#diff-40> (12)
M test/SILGen/default_arguments_imported.swift <https://github.com/apple/swift/pull/12260/files#diff-41> (6)
M test/SILGen/dynamic.swift <https://github.com/apple/swift/pull/12260/files#diff-42> (94)
M test/SILGen/dynamic_self.swift <https://github.com/apple/swift/pull/12260/files#diff-43> (2)
M test/SILGen/extensions_objc.swift <https://github.com/apple/swift/pull/12260/files#diff-44> (6)
M test/SILGen/foreign_errors.swift <https://github.com/apple/swift/pull/12260/files#diff-45> (18)
M test/SILGen/inlineable_attribute_objc.swift <https://github.com/apple/swift/pull/12260/files#diff-46> (2)
M test/SILGen/objc_attr_NSManaged.swift <https://github.com/apple/swift/pull/12260/files#diff-47> (14)
M test/SILGen/objc_attr_NSManaged_multi.swift <https://github.com/apple/swift/pull/12260/files#diff-48> (12)
M test/SILGen/objc_blocks_bridging.swift <https://github.com/apple/swift/pull/12260/files#diff-49> (6)
M test/SILGen/objc_bool_bridging.swift <https://github.com/apple/swift/pull/12260/files#diff-50> (24)
M test/SILGen/objc_bridged_results.swift <https://github.com/apple/swift/pull/12260/files#diff-51> (18)
M test/SILGen/objc_bridging.swift <https://github.com/apple/swift/pull/12260/files#diff-52> (36)
M test/SILGen/objc_bridging_any.swift <https://github.com/apple/swift/pull/12260/files#diff-53> (52)
M test/SILGen/objc_currying.swift <https://github.com/apple/swift/pull/12260/files#diff-54> (6)
M test/SILGen/objc_dealloc.swift <https://github.com/apple/swift/pull/12260/files#diff-55> (2)
M test/SILGen/objc_extensions.swift <https://github.com/apple/swift/pull/12260/files#diff-56> (16)
M test/SILGen/objc_factory_init.swift <https://github.com/apple/swift/pull/12260/files#diff-57> (6)
M test/SILGen/objc_imported_generic.swift <https://github.com/apple/swift/pull/12260/files#diff-58> (8)
M test/SILGen/objc_init_ref_delegation.swift <https://github.com/apple/swift/pull/12260/files#diff-59> (2)
M test/SILGen/objc_nonnull_lie_hack.swift <https://github.com/apple/swift/pull/12260/files#diff-60> (8)
M test/SILGen/objc_ownership_conventions.swift <https://github.com/apple/swift/pull/12260/files#diff-61> (18)
M test/SILGen/objc_properties.swift <https://github.com/apple/swift/pull/12260/files#diff-62> (14)
M test/SILGen/objc_subscript.swift <https://github.com/apple/swift/pull/12260/files#diff-63> (8)
M test/SILGen/objc_super.swift <https://github.com/apple/swift/pull/12260/files#diff-64> (8)
M test/SILGen/objc_thunks.swift <https://github.com/apple/swift/pull/12260/files#diff-65> (20)
M test/SILGen/objc_witnesses.swift <https://github.com/apple/swift/pull/12260/files#diff-66> (6)
M test/SILGen/properties.swift <https://github.com/apple/swift/pull/12260/files#diff-67> (2)
M test/SILGen/super_objc_class_method.swift <https://github.com/apple/swift/pull/12260/files#diff-68> (2)
M test/SILGen/vtables_objc.swift <https://github.com/apple/swift/pull/12260/files#diff-69> (4)
M test/SILOptimizer/cse_objc.sil <https://github.com/apple/swift/pull/12260/files#diff-70> (2)
M test/SILOptimizer/definite_init_objc_factory_init.swift <https://github.com/apple/swift/pull/12260/files#diff-71> (6)
M test/SILOptimizer/devirt_jump_thread.sil <https://github.com/apple/swift/pull/12260/files#diff-72> (2)
M test/SILOptimizer/inlinecaches_objc.sil <https://github.com/apple/swift/pull/12260/files#diff-73> (2)
M test/SILOptimizer/looprotate.sil <https://github.com/apple/swift/pull/12260/files#diff-74> (2)
M test/SILOptimizer/outliner.swift <https://github.com/apple/swift/pull/12260/files#diff-75> (10)
M test/SILOptimizer/simplify_cfg.sil <https://github.com/apple/swift/pull/12260/files#diff-76> (2)
M test/SILOptimizer/super_objc_class_method.swift <https://github.com/apple/swift/pull/12260/files#diff-77> (2)
M test/Serialization/objc.swift <https://github.com/apple/swift/pull/12260/files#diff-78> (12)
Patch Links:
https://github.com/apple/swift/pull/12260.patch <https://github.com/apple/swift/pull/12260.patch>
https://github.com/apple/swift/pull/12260.diff <https://github.com/apple/swift/pull/12260.diff>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#12260>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAee34vfq5C7lA1iDVsY5TClffgmUfV3ks5sovvcgaJpZM4PtDbd>.
|
This is the first part of a refactoring to make Objective-C dispatch explicit in SIL, instead of relying on a poorly-named 'volatile' bit in the class_method / super_method / witness_method instructions.
The next step is to switch Objective-C protocol method calls over to use objc_method, since witness_method models Objective-C dispatch poorly.
The reason I'm doing this is because it is a general cleanup we've talked about for a while, and also because we might switch to a resilient method dispatch strategy where it would be advantageous to change class_method to take an isa pointer, and replace super_method with a super_metatype + class_method sequence. Since this separation doesn't make sense for Objective-C, I'm going to land this refactoring first.
Also, it might be possible to eliminate the dynamic_method instruction entirely now, but I haven't looked into that yet.
Unfortunately the new instructions do introduce a lot of new code, but some upcoming changes I'm planning on making to SILGen will eliminate the duplication for lowering call expressions a fair bit.