Last Comment Bug 610323 - [TraceMonkey] Implement Jaegermonkey Sparc back-end.
: [TraceMonkey] Implement Jaegermonkey Sparc back-end.
Status: RESOLVED FIXED
fixed-in-tracemonkey
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: Sun Solaris
: -- normal (vote)
: ---
Assigned To: Leon Sha
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-08 00:19 PST by Leon Sha
Modified: 2011-04-26 15:41 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (124.91 KB, patch)
2010-11-08 00:19 PST, Leon Sha
no flags Details | Diff | Review
Patch to add PolyIC support (153.93 KB, patch)
2010-11-16 19:32 PST, Leon Sha
no flags Details | Diff | Review
Patch v2 with David's comments. (154.35 KB, patch)
2010-12-02 17:21 PST, Leon Sha
no flags Details | Diff | Review
New file to be added. (104.29 KB, patch)
2011-03-29 22:52 PDT, Leon Sha
dvander: review+
Details | Diff | Review
Patch part 2 (47.37 KB, patch)
2011-03-29 22:53 PDT, Leon Sha
dvander: review+
Details | Diff | Review
patch part 2 v2 (47.66 KB, patch)
2011-04-18 01:31 PDT, Leon Sha
dvander: review+
Details | Diff | Review

Description Leon Sha 2010-11-08 00:19:41 PST
Created attachment 488819 [details] [diff] [review]
patch

Need to implement Jaegermonkey Sparc back-end.
Comment 1 Leon Sha 2010-11-16 19:32:51 PST
Created attachment 491082 [details] [diff] [review]
Patch to add PolyIC support
Comment 2 David Anderson [:dvander] 2010-12-01 19:10:14 PST
Comment on attachment 491082 [details] [diff] [review]
Patch to add PolyIC support

Not done reviewing yet, but here's some initial comments...

>+    static void cacheFlush(void* code, size_t size)
>+    {
>+		  	caddr_t v = (caddr_t)code;
>+		  	caddr_t end = v + size;
>+				caddr_t p = v;
>+				while (p < end) {
>+					  asm("flush %0" : : "r" (p));
>+						p += 32;
>+				}
>+		}

Not sure what happened with spacing here.

>+#if WTF_CPU_BIG_ENDIAN
>+    static const uint32 PAYLOAD_OFFSET = 4;
>+    static const uint32 TAG_OFFSET     = 0;
>+#else
>     static const uint32 PAYLOAD_OFFSET = 0;
>     static const uint32 TAG_OFFSET     = 4;
>+#endif

Use IS_BIG_ENDIAN here to avoid WTF_* leaking outside of assembler.

>+#if defined JS_CPU_X86 || defined JS_CPU_ARM || defined JS_CPU_SPARC
>         static const int CanonicalNaNType = 0x7FF80000;
>         masm.setPtr(oppositeCond, treg, Imm32(CanonicalNaNType), result);
> #elif defined JS_CPU_X64
>         static const void *CanonicalNaNType = (void *)0x7FF8000000000000; 
>         masm.move(ImmPtr(CanonicalNaNType), JSC::X86Registers::r11);
>         masm.setPtr(oppositeCond, treg, JSC::X86Registers::r11, result);
> #endif

#ifndef JS_64BIT ... #else should be good enough here. Bonus points if you change that r11 to be scratchRegister.

>+#if WTF_CPU_BIG_ENDIAN
>+    static const uint32 POLY_PAYLOAD_OFFSET = 4;
>+    static const uint32 POLY_TAG_OFFSET     = 0;
>+#else
>+    static const uint32 POLY_PAYLOAD_OFFSET = 0;
>+    static const uint32 POLY_TAG_OFFSET     = 4;

Why do we need to dupliate this from BaseAssembler?

CC'ing Chris Leary for the regex changes.
Comment 3 Leon Sha 2010-12-02 17:21:32 PST
Created attachment 494895 [details] [diff] [review]
Patch v2 with David's comments.

Make some changes according to David's Comments. Also I changed some bit value type from int to int32. Also I expand the size of "secondShapeGuard" to 11 bits since 8 bits is not enough for Sparc.
Comment 4 Wesley W. Garland 2011-03-25 09:01:41 PDT
I know it's not my bug but -- review ping?

I'd like to see this get merged in with the mainline before it rots. According to these SunSpiders numbers, it's already shipping as a contrib build:

Both:           1653
TM Only:        2012
JM Only:        1769
Interp Only:    7015

(SunFire v240, lightly loaded, 2x1503MHz cores, 4096MB physmem, 5.10 in a sparse-root zone)
Comment 5 Leon Sha 2011-03-29 22:52:57 PDT
Created attachment 522932 [details] [diff] [review]
New file to be added.
Comment 6 Leon Sha 2011-03-29 22:53:42 PDT
Created attachment 522933 [details] [diff] [review]
Patch part 2
Comment 7 David Anderson [:dvander] 2011-04-15 18:24:52 PDT
Comment on attachment 522933 [details] [diff] [review]
Patch part 2

Sorry about the delay, and thanks for fixing all the payloadOf() misuses!

>             // obj->getFlatClosureUpvars()
>             masm.loadPtr(Address(reg, offsetof(JSObject, slots)), reg);
>             Address upvarAddress(reg, JSObject::JSSLOT_FLAT_CLOSURE_UPVARS * sizeof(Value));
>-            masm.loadPrivate(upvarAddress, reg);
>+            masm.loadPrivate(masm.payloadOf(upvarAddress), reg);

loadPrivate() always takes a Value*, so I would move the payloadOf() call into loadPrivate().

r=me with that
Comment 8 Leon Sha 2011-04-18 01:31:32 PDT
Created attachment 526677 [details] [diff] [review]
patch part 2 v2

I put the payloadof inside loadprivate. Also I change the cacheFlush to be the same as flushICache in nanojit.

Note You need to log in before you can comment on or make changes to this bug.