Bug 63893 - Make "Add optimised paths for a few maths functions" work on Qt
Summary: Make "Add optimised paths for a few maths functions" work on Qt
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 63757
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-04 01:38 PDT by Zoltan Herczeg
Modified: 2011-07-05 18:20 PDT (History)
6 users (show)

See Also:


Attachments
patch (3.04 KB, patch)
2011-07-04 02:00 PDT, Zoltan Herczeg
no flags Details | Formatted Diff | Diff
updated patch (2.84 KB, patch)
2011-07-05 02:23 PDT, Zoltan Herczeg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.