Bug 63893

Summary: Make "Add optimised paths for a few maths functions" work on Qt
Product: WebKit Reporter: Zoltan Herczeg <zherczeg>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, barraclough, eric, loki, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 63757    
Bug Blocks:    
Attachments:
Description Flags
patch
none
updated patch none

Description Zoltan Herczeg 2011-07-04 01:38:36 PDT
Make it works!
Comment 1 Zoltan Herczeg 2011-07-04 02:00:08 PDT
Created attachment 99610 [details]
patch
Comment 2 Gabor Loki 2011-07-04 02:10:18 PDT
Comment on attachment 99610 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99610&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Move the generated code to the .text section instead of .data section.

Nice catch!

Would it make sense to try this optimization on ARM?
Comment 3 Zoltan Herczeg 2011-07-04 06:32:07 PDT
> Would it make sense to try this optimization on ARM?

We need to detect whether hard or softfp ABI is present. Hopefully the compiler will tell us...
Comment 4 Oliver Hunt 2011-07-04 09:26:46 PDT
Comment on attachment 99610 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99610&action=review

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:142
> +        "jmp " SYMBOL_STRING_RELOCATION(function) "\n" \

Changing this to a jump will misalign the stack on x86_32/mac are you sure it's not complaining about it?
Comment 5 Zoltan Herczeg 2011-07-04 09:46:56 PDT
> Changing this to a jump will misalign the stack on x86_32/mac are you sure it's not complaining about it?

Misalign? Why? Keeps 8 byte alignment, even if you don't push an extra 8 byte return address to it. Or is that 16 byte aligned? I don't know about mac ABI.
Comment 6 Oliver Hunt 2011-07-04 11:55:35 PDT
(In reply to comment #5)
> > Changing this to a jump will misalign the stack on x86_32/mac are you sure it's not complaining about it?
> 
> Misalign? Why? Keeps 8 byte alignment, even if you don't push an extra 8 byte return address to it. Or is that 16 byte aligned? I don't know about mac ABI.

Mac ABI requires 16byte alignment
Comment 7 Zoltan Herczeg 2011-07-04 12:33:35 PDT
> Mac ABI requires 16byte alignment

thanks for letting me know. Tomorrow I will put it back and resubmit the patch.
Comment 8 Zoltan Herczeg 2011-07-05 02:23:13 PDT
Created attachment 99685 [details]
updated patch
Comment 9 Eric Seidel (no email) 2011-07-05 18:08:46 PDT
Seems reasonable to me, but the real JSC experts shoudl comment.
Comment 10 WebKit Review Bot 2011-07-05 18:20:39 PDT
Comment on attachment 99685 [details]
updated patch

Clearing flags on attachment: 99685

Committed r90425: <http://trac.webkit.org/changeset/90425>
Comment 11 WebKit Review Bot 2011-07-05 18:20:44 PDT
All reviewed patches have been landed.  Closing bug.