EABI requies that 64-bit integer when being passed as argument must start at an even number register (r0-r1 or r2-r3). But current DFG JIT doesn't take care of this, and assumes 64-bit integer can be passed through r1, r2. I have a patch to work around this issue (will submit it soon). But that looks ugly. Any idea about a neat solution? Also I don't see DFG boosts performance on sunspider. Actually DFG gives worse result. Any idea about this?
Created attachment 138093 [details] the ugly work around EncodedJSValue is 64-bit integer for JSVALUE32_64. When being passed as argument, an EncodedJSValue is aligned to r0 or r2. iOS may have its own EABI... One solution may be splitting EncodedJSValue arguments into 32-bit integers for those DFG operation calls.
Interesting, looking at the iOS developer docs, this may be a difference on iOS: http://developer.apple.com/library/ios/#documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARMv6FunctionCallingConventions.html#//apple_ref/doc/uid/TP40009021-SW1 * Large data types (larger than 4 bytes) are 4-byte aligned. We don't want to have to duplicate all these method to have two signatures, I guess we could provide a set of wrappers on non-iOS ARM, but the cleanest approach may just be to fix the callOperation interface to align the arguments correctly. If this degrades performance (by moving augments from registers to the stack) we can always change the order of args to functions affected to compensate.
(In reply to comment #2) > Interesting, looking at the iOS developer docs, this may be a difference on iOS: > > http://developer.apple.com/library/ios/#documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARMv6FunctionCallingConventions.html#//apple_ref/doc/uid/TP40009021-SW1 > > * Large data types (larger than 4 bytes) are 4-byte aligned. > > We don't want to have to duplicate all these method to have two signatures, I guess we could provide a set of wrappers on non-iOS ARM, but the cleanest approach may just be to fix the callOperation interface to align the arguments correctly. If this degrades performance (by moving augments from registers to the stack) we can always change the order of args to functions affected to compensate. So how about the patch?
(In reply to comment #3) > (In reply to comment #2) > > Interesting, looking at the iOS developer docs, this may be a difference on iOS: > > > > http://developer.apple.com/library/ios/#documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARMv6FunctionCallingConventions.html#//apple_ref/doc/uid/TP40009021-SW1 > > > > * Large data types (larger than 4 bytes) are 4-byte aligned. > > > > We don't want to have to duplicate all these method to have two signatures, I guess we could provide a set of wrappers on non-iOS ARM, but the cleanest approach may just be to fix the callOperation interface to align the arguments correctly. If this degrades performance (by moving augments from registers to the stack) we can always change the order of args to functions affected to compensate. > > So how about the patch? I like the general approach of your patch. Let me know when it's ready to be reviewed.
(In reply to comment #4) > (In reply to comment #3) > > > > So how about the patch? > > I like the general approach of your patch. Let me know when it's ready to be reviewed. Thanks. Sure I'll upload a ready-for-review patch.
Created attachment 139506 [details] the patch
Comment on attachment 139506 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=139506&action=review > Source/JavaScriptCore/dfg/DFGOperations.cpp:102 > +// EncodedJSValue in JSVALUE32_64 is a 64-bit integer. When being compiled in ARM EABI, it must be aligned even-numbered register (r0, r2 or [sp]). Hm... should be "aligned to"
Comment on attachment 139506 [details] the patch R=me. Sorry it took so long to review.
*** Bug 86335 has been marked as a duplicate of this bug. ***
Comment on attachment 139506 [details] the patch Thanks!
Comment on attachment 139506 [details] the patch Clearing flags on attachment: 139506 Committed r116951: <http://trac.webkit.org/changeset/116951>
All reviewed patches have been landed. Closing bug.