Closed
Bug 452390
Opened 16 years ago
Closed 16 years ago
Tracemonkey will crash if the compiler doesn't have FASTCALL
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)
References
Details
Attachments
(2 files, 1 obsolete file)
|
4.58 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
|
1.63 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
Microsoft or GCC __fastcall convention (aka __msfastcall) passes the first two arguments (evaluated left to right) that fit into ECX and EDX. If I'm using another compiler which doesn't have FASTCALL, the function generated by Tracemonkey still tries to get value from ECX and EDX. IMHO, we should either disable JIT or deal with it in nanojit.
There are seriously compilers that don't give you a way to do gcc/msvc fastcall? I'm pretty surprised! I would recommend patching nanojit to use another optimized calling convention, then, and submitting it upstream with tests.
I hacked nanojit. It's not too hard. Now it works for me on Solaris x86. The SunSpider result is 1.76x as fast. I'll submit patches soon.
I used a GCC style inline asm to simulate a fastcall to u.func. Also push ECX, EDX when calling back to jsbuiltin functions. I also tested it on Mac OS X. (modified jstypes.h to turn off fastcall) trace-test.js passed with sse2 features.
Same patch, a few Nit changes. Sorry for my spam. BTW: Please ignore "v3". This is the initial version.
Attachment #336341 -
Attachment is obsolete: true
Attachment #336342 -
Flags: review?(shaver)
Attachment #336341 -
Flags: review?(shaver)
Comment on attachment 336342 [details] [diff] [review] patch I'm not a good choice for review here; since the majority of the changes appear to be in nanojit, I'd recommend David Anderson to start. (I'm not generally a good choice for review these days without some previous communication about the patch, due to my workload.)
Attachment #336342 -
Flags: review?(shaver) → review?(danderson)
|
||
Updated•16 years ago
|
Attachment #336342 -
Flags: review?(danderson) → review+
Cool patch, pushed to tracemonkey repo as changeset de2d26b3c902. I made a few little style changes to keep the nanojit/TraceMonkey separation. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
|
||
Updated•16 years ago
|
Flags: in-testsuite-
Flags: in-litmus-
It's not working any longer.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #347500 -
Flags: review?(danderson)
|
|
||
Updated•16 years ago
|
Attachment #347500 -
Flags: review?(danderson) → review+
pushed to tracemonkey http://hg.mozilla.org/tracemonkey/rev/9caa6acaeee0
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•