514632 - Use Sun Studio inline asm template for NativeCompareAndSwap

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ginn Chen]

In Bug 502696, we use gcc style inline asm for NativeCompareAndSwap with Sun Studio.

I just found current Sun Studio SPARC is buggy for compiling gcc style asm.
It doesn't work if optimization level is low.

So I have to rewrite it to Sun Studio inline asm template.

I also implemented sparcv9  64bit version in this patch.

Comment 1

[Ginn Chen]

Attachment: patch

ASFLAGS is removed, since we're not going to use AS here.

lock_sparcv8plus.il is based on lock_SunOS.s ( I forgot to remove the file in Bug 502696.)
The different of sparcv9 and sparcv8plus is casx vs cas.
I've to copy the file because I don't find a way to use ifdef in inline template.

It would be more efficient comparing to lock_SunOS.s version, because we can save a function call.
A copy of NativeCompareAndSwap would be inserted in place.

Comment 2

[Wesley W. Garland]

Questions --

1 - Does this style work with GCC?
2 - How do you differentiate between v9 and v8+ builds?
3 - Does it not make more sense to modify the Makefile and increase the optimization level for jslock.cpp on SunStudio, rather than branching the source code to work around a compiler bug?

Also - FWIW - I can't stamp this, you need a JS peer for that.  I will try and find some time to test it soon, though. My compilation env is gcc3.4.3 (/usr/sfw), v8+.

Comment 3

[Ginn Chen]

1. No.

2. In Makefile.in, if OS_TEST is sparcv9, it is 64-bit, if OS_TEST is sparc, it is v8+ 32-bit.

3. It is not yet supported by Sun Studio 12/12u1, although it seams it works OK with xO2 or above now.
But I'm afraid it's fragile. It might be broken by code changes or compiler updates.
I think we should avoid it until it is officially supported.

Comment 4

[Jeff Walden [:Waldo]]

(In reply to comment #2)
> Also - FWIW - I can't stamp this, you need a JS peer for that.

Righto, but Mozilla review policies have never recognized the nondelegation doctrine.  I don't know which of us is actually familiar with Solaris assembly, but if none of us are it's quite likely we'd ask someone who did to help (or recognize such a review if asked after the errant review had already been performed).

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 398621 [details] [diff] [review]
patch

This looks fine. If you don't post a new one in a day or two, I'll push it.

It seems weird to have a .inline that the C compiler sees as an 'extern "C"' declaration.  Out of curiosity, can the compiler really inline it in that case? Link-time code generation maybe?

Comment 6

[Ginn Chen]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/def6be3d8b6b

Yes, the asm snippet is inlined no matter debug version or release version.

Comment 7

[Ginn Chen]

Really remove the unused file.
http://hg.mozilla.org/mozilla-central/rev/9ecbae6faef1