Closed Bug 587962 Opened 10 years ago Closed 10 years ago

[JAEGER] JaegerMonkey build and check failed on Solaris x86 with Sun Studio 12

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Solaris
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: leon.sha, Assigned: leon.sha)

Details

Attachments

(3 files, 1 obsolete file)

patch v2
10.73 KB, patch
dvander
: review+
Details | Diff | Splinter Review
patch
2.78 KB, patch
dvander
: review+
Details | Diff | Splinter Review
Patch 4
753 bytes, patch
dvander
: review+
Details | Diff | Splinter Review
Attached patch patch (obsolete) — — Splinter Review 
The latest several commits break JaegerMonkey on Solaris x86 with Sun Studio 12.
The patch include the following fixes.
1. Disable YARR_CALL for sunstudio.
2. Modify TrampolineSUNWX86.s according the latest changes.
3. Fix a fast call related problem in js_InternalThrow.
4. Fix a build error on sunstudio.
Attachment #466582 - Attachment is patch: true
Attachment #466582 - Attachment mime type: application/octet-stream → text/plain
Attachment #466582 - Flags: review?(dvander)
Comment on attachment 466582 [details] [diff] [review]
patch


>+            // trampolines.forceReturn added esp by 8 to balance the esp
>+            // for fastcall. js_InternalThrow is not a fast call.
>+            // We need to skip this add esp instruction using a hack way.
>+            forcereturn = (void *)((int32_t)forcereturn + 3);
>+#endif

Is it possible to change the forceReturn trampoline instead of this hack?
(In reply to comment #1)
> Comment on attachment 466582 [details] [diff] [review]
> patch
> 
> 
> >+            // trampolines.forceReturn added esp by 8 to balance the esp
> >+            // for fastcall. js_InternalThrow is not a fast call.
> >+            // We need to skip this add esp instruction using a hack way.
> >+            forcereturn = (void *)((int32_t)forcereturn + 3);
> >+#endif
> 
> Is it possible to change the forceReturn trampoline instead of this hack?

forceReturn is used by both js_InternalThrow and some fast calls. Another solution is to add a forceReturnFast similar as forceReturn.
Attached patch patch v2 — — Splinter Review 
This patch didn't do hack things. But there are more changes.
Assignee: general → leon.sha
Status: NEW → ASSIGNED
Attachment #466933 - Flags: review?(dvander)
Comment on attachment 466933 [details] [diff] [review]
patch v2

Sorry for delay.
Attachment #466933 - Flags: review?(dvander) → review+
Attachment #466582 - Attachment is obsolete: true
Attachment #466582 - Flags: review?(dvander)
http://hg.mozilla.org/projects/jaegermonkey/rev/c2f1e5150e18
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attached patch patch — — Splinter Review 
Since the patch for bug 587833 lunched, we need to modify solaris trampolines too.
Attachment #468264 - Flags: review?(dvander)
Attachment #468264 - Flags: review?(dvander) → review+
Attached patch Patch 4 — — Splinter Review 
Some trap test failed. The code for non-fastcall missing during merge.
Attachment #472520 - Flags: review?(dvander)
Comment on attachment 472520 [details] [diff] [review]
Patch 4

Leon, the JM tree is now for debugging/testing only. The patch that caused this problem hasn't landed on TM yet.

I'll make sure to integrate your fix into the final check-in. Thanks!
Attachment #472520 - Flags: review?(dvander) → review+
(In reply to comment #11)
> Comment on attachment 472520 [details] [diff] [review]
> Patch 4
> 
> Leon, the JM tree is now for debugging/testing only. The patch that caused this
> problem hasn't landed on TM yet.
> 
> I'll make sure to integrate your fix into the final check-in. Thanks!

Thank you for noticing. I'll also monitor the TM tree.
You need to log in before you can comment on or make changes to this bug.