[ffigen] Only use objc_msgSend variants on x64
#836
Conversation
There was a problem hiding this comment.
Thanks @liamappelbe!
I think this code could use some simplification before landing.
| @@ -35168,25 +35768,46 @@ class NSString extends NSObject { | |||
|
|
|||
| void localizedStandardRangeOfString_( | |||
| ffi.Pointer<_NSRange> stret, NSString str) { | |||
| _lib._objc_msgSend_340( | |||
| stret, _id, _lib._sel_localizedStandardRangeOfString_1, str._id); | |||
| _lib._objc_msgSend_useVariants1 | |||
There was a problem hiding this comment.
Note: This will probs also need treatment in #216.
There was a problem hiding this comment.
Are @Native annotations lazy loaded? If so this approach should work.
There was a problem hiding this comment.
Yes, they are "dynamic loading" everywhere currently.
If we'd ever to static linking, it would be after TFA. @sstrickl has been working passing the OS in to TFA, when I get to static linking, I could extend that work to cover Abi.
pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart
Outdated
Show resolved
Hide resolved
| @@ -417,28 +427,81 @@ enum ObjCMsgSendVariant { | |||
| /// A wrapper around the objc_msgSend function, or the stret or fpret variants. | |||
| class ObjCMsgSendFunc { | |||
| final ObjCMsgSendVariant variant; | |||
| late final Func func; | |||
| final ObjCInternalGlobal useVariants; | |||
There was a problem hiding this comment.
This could use documentation. Ditto for the other fields, and maybe a larger explanation on ObjCMsgSendFunc itself.
What I've been able to reverse engineer:
isStretwhether ifuseVariantis true, thestretvariant is used. Requires a pointer to the struct as extra param.useVariantwhether at runtime the ABI is x64 on iOS or MacOS.variantFuncthestretorfpretvariants. null if only normal func.normalFuncalways the normal variant.
We emit both variant and normalFunc and branch at runtime based on ABI.
The same documentation should go on the PR description.
| bool get isStret => variant == ObjCMsgSendVariant.stret; | ||
| bool get isNormal => variant == ObjCMsgSendVariant.normal; |
There was a problem hiding this comment.
Hm, maybe the code would be more readable if the call sites had switches over the enum. (e.g. !isNormal would be both the stret and fpret cases.)
| final stretCall = | ||
| _invoke(variantFunc!.name, lib, target, sel, params, stret: stret); | ||
| return '$lib.${useVariants.name} ? $stretCall : $stret.ref = $normalCall'; | ||
| } else if (variantFunc != null) { |
There was a problem hiding this comment.
This is not so readable: variantFunc != null if !isNormal. Since we already covered isStret, this is the fpret case.
Please convert this if else if else to a switch on variant with the 3 cases.
objc_msgSend_stretandobjc_msgSend_fpretare only available on x64, so just use the normalobjc_msgSendon arm64.We can only check the ABI at runtime, so for the stret and fpret variants we need to emit both the variant and the normal
objc_msgSendfunction. Then we can decide which to use when the method is invoked at runtime.This runtime check is complicated by the fact that
objc_msgSend_strethas a different signature thanobjc_msgSendhas for the same method. This is becauseobjc_msgSend_strettakes a pointer to the return type as its first arg. If it wasn't for this signature difference we could just change the function name string we pass toDynamicLibrary.lookup.Fixes #829