Closed
Bug 655260
Opened 13 years ago
Closed 13 years ago
Burning JaegerMonkey (TI) for ARM.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jbramley, Assigned: jbramley)
Details
Attachments
(5 files, 2 obsolete files)
2.07 KB,
patch
|
paul.biggar
:
review+
|
Details | Diff | Splinter Review |
8.20 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
4.33 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
646 bytes,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
4.71 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
There are a few small patches required here, so I've just created one bug to reduce Bugzilla noise. Most of these are probably the result of merges and new development on the type-inference branch.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #530630 -
Flags: review?(pbiggar)
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #530631 -
Flags: review?(pbiggar)
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #530632 -
Flags: review?(pbiggar)
Assignee | ||
Comment 4•13 years ago
|
||
We aren't there yet. There's another failure after adding these three: /work/moz/mozilla.org/projects/jaegermonkey/js/src/methodjit/FastOps.cpp: In member function 'void js::mjit::Compiler::jsop_stricteq(JSOp)': /work/moz/mozilla.org/projects/jaegermonkey/js/src/methodjit/FastOps.cpp:1932: error: 'class js::mjit::FrameState' has no member named 'pushSyncedType'
Comment 5•13 years ago
|
||
Comment on attachment 530630 [details] [diff] [review] 1: Tweak register name calls on ARM to fix the build. Review of attachment 530630 [details] [diff] [review]: ----------------------------------------------------------------- That's all? Great!
Attachment #530630 -
Flags: review?(pbiggar) → review+
Comment 6•13 years ago
|
||
Comment on attachment 530631 [details] [diff] [review] 2: Adjust VMFrame assertions to fix the ARM build. Review of attachment 530631 [details] [diff] [review]: ----------------------------------------------------------------- Don't know this code, gonna leave it to Brian.
Attachment #530631 -
Flags: review?(pbiggar) → review?(bhackett1024)
Comment 7•13 years ago
|
||
Comment on attachment 530632 [details] [diff] [review] 3: Add absDouble to the ARM back-end. The 'x & -x' trick doesn't look sensible on ARM. Review of attachment 530632 [details] [diff] [review]: ----------------------------------------------------------------- Again, not familiar here, so I'm gonna defer to Brian. Id say abstract the x&-x into an absdouble method on other platforms, but don't know if that's appropriate.
Attachment #530632 -
Flags: review?(pbiggar) → review?(bhackett1024)
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to comment #7) > Id say abstract the x&-x into an absdouble method on other platforms, but > don't know if that's appropriate. Yep, that would certainly be cleaner!
Updated•13 years ago
|
Attachment #530631 -
Flags: review?(bhackett1024) → review+
Updated•13 years ago
|
Attachment #530632 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 9•13 years ago
|
||
I had to make a couple of changes to this as a few things got moved around in the repository. It probably doesn't need another full review, but I've got a couple more for review anyway.
Attachment #530631 -
Attachment is obsolete: true
Attachment #531599 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 10•13 years ago
|
||
The only change here is that I added absDouble to the x86 back-end, as suggested.
Attachment #530632 -
Attachment is obsolete: true
Attachment #531601 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 11•13 years ago
|
||
This one really is trivial. It looks like the pushSyncedType method was renamed to overload the pushSynced method. This patch just updates a call site accordingly.
Attachment #531603 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 12•13 years ago
|
||
This is the only non-trivial patch required to fix the ARM build. I'm not entirely sure that my interpoline code is correct, but it seems to do everything that the x86 version does. This is the last patch required to make the type inference tree build on ARM. It still fails several tests, but they'll get separate bugs (and patches).
Attachment #531604 -
Flags: review?(bhackett1024)
Updated•13 years ago
|
Attachment #531599 -
Flags: review?(bhackett1024) → review+
Updated•13 years ago
|
Attachment #531601 -
Flags: review?(bhackett1024) → review+
Updated•13 years ago
|
Attachment #531603 -
Flags: review?(bhackett1024) → review+
Comment 13•13 years ago
|
||
Comment on attachment 531604 [details] [diff] [review] 5: Add interpoline support to ARM. Review of attachment 531604 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks! ::: js/src/methodjit/MethodJIT.cpp @@ +609,5 @@ > +SYMBOL_STRING(JaegerInterpolineScripted) ":" "\n" > + /* The only difference between JaegerInterpoline and JaegerInpolineScripted is that the > + * scripted variant has to walk up to the previous StackFrame first. */ > +" ldr r11, [r11, #(4*4)]" "\n" /* Load f->prev_ */ > +" str r11, [sp, #(4*7)]" "\n" /* Update VMFrame->entryFP */ Comment should be updating f.regs.fp (the offset itself looks right). @@ +621,5 @@ > +" mov r1, r5" "\n" /* returnType */ > +" mov r0, r4" "\n" /* returnData */ > +" blx " SYMBOL_STRING_RELOC(js_InternalInterpret) "\n" > +" cmp r0, #0" "\n" > +" ldr ip, [sp, #(4*7)]" "\n" /* Load (StackFrame*)entryFp */ Ditto.
Attachment #531604 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to comment #13) > Comment should be updating f.regs.fp (the offset itself looks right). Ah, yes, I thought that was odd. Thanks! (I'd counted FrameRegs as a single field.) http://hg.mozilla.org/projects/jaegermonkey/rev/e96dad5f95fd http://hg.mozilla.org/projects/jaegermonkey/rev/07270a0cbc86 http://hg.mozilla.org/projects/jaegermonkey/rev/5e9fa1b150aa http://hg.mozilla.org/projects/jaegermonkey/rev/43c304c90149 http://hg.mozilla.org/projects/jaegermonkey/rev/ef221c659ef0
Assignee | ||
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•