Closed Bug 502369 Opened 11 years ago Closed 10 years ago

sparc linux lacks sync_instruction_memory rendering xulrunner unbuildable

Categories

(Core Graveyard :: Nanojit, defect)

Sun
Linux
defect
Not set
major

Tracking

(status1.9.2 .13-fixed, status1.9.1 wontfix)

RESOLVED FIXED
flash10.1
Tracking Status
status1.9.2 --- .13-fixed
status1.9.1 --- wontfix

People

(Reporter: dennis, Assigned: dennis)

References

Details

(Whiteboard: Has patch, fixed-in-tamarin)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4) Gecko/20090427 Fedora/3.5-0.20.beta4.fc11 Firefox/3.5b4
Build Identifier: 

sparclinux libc does not have sync_instruction_memory  the attached patch add support for sparc linux  so it will again be buildable. 

Reproducible: Always

Steps to Reproduce:
1. compile 
2.
3.
Actual Results:  
build fails with missing symbol

Expected Results:  
compilation to succeed
Assignee: general → dennis
Status: UNCONFIRMED → NEW
Component: JavaScript Engine → JIT Compiler (NanoJIT)
Ever confirmed: true
Product: Core → Tamarin
QA Contact: general → nanojit
Attachment #386822 - Flags: review?(jim)
Attachment #386822 - Flags: review?(gal)
Not sure if this is the right component or not...
Attachment #386822 - Flags: review?(gal) → review+
Comment on attachment 386822 [details] [diff] [review]
patch to add sync_instruction_memory

Ed, you guys might want this too.
Attachment #386822 - Flags: review?(jim) → review?(edwsmith)
Blocks: 502642
Comment on attachment 386822 [details] [diff] [review]
patch to add sync_instruction_memory

added tamarin bug 502642 to track this one
Attachment #386822 - Flags: review?(edwsmith) → review+
Duplicate of this bug: 486584
Comment on attachment 386822 [details] [diff] [review]
patch to add sync_instruction_memory

>+		asm("flush %0" : : "r" (p));

IMHO, this should be made volatile. And iflush might be more appropriate than flush, if I'm not mistaken.
Someone mentioned here using iflush
https://bugzilla.mozilla.org/show_bug.cgi?id=486584#c2
And in solaris code flush is used.
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/sparc/gen/sync_instruction_memory.s
I think both is OK.

>+		p += 32;
I don't understand why it is 32 here. Should be 8.
(In reply to comment #6)
> (From update of attachment 386822 [details] [diff] [review])
> >+		asm("flush %0" : : "r" (p));
> 
> IMHO, this should be made volatile. And iflush might be more appropriate than
> flush, if I'm not mistaken.

In the Sparc v8 and v9 manuals it is called "flush"; it was called "iflush" in v7.  Presumably the old name lives on in various assemblers.
Nobody will ever make a sparc V9 chip with an I-cache with line sizes
smaller than 32-bytes.

Doing it 8 bytes at a time is simply a bunch of wasted full pipeline flushes.

That's why we iterate 32-bytes at a time.
(In reply to comment #9)
> Nobody will ever make a sparc V9 chip with an I-cache with line sizes
> smaller than 32-bytes.

Is spidermonkey targetted at v9+ only, now ?
If you find even one person running this code on a V8 or earlier
Sparc processor, I will buy you a multi-million dollar penthouse
condo overlooking Central Park in New York City.
(In reply to comment #11)
> If you find even one person running this code on a V8 or earlier
> Sparc processor, I will buy you a multi-million dollar penthouse
> condo overlooking Central Park in New York City.

Well this machine is only $49.

http://cgi.ebay.com/Sun-SPARCstation-5-96MB-microSPARC-II-CD-ROM-FDD_W0QQitemZ390063492867QQcmdZViewItemQQptZLH_DefaultDomain_0?hash=item5ad1988f03&_trksid=p3286.c0.m14&_trkparms=65%3A12|66%3A2|39%3A1|72%3A1234|293%3A1|294%3A50

Will you cover moving costs?
@Mike & David: I have proposed deprecating SPARC V8 support entirely as part of the fix for bug 502696.

@Robert: The fastest SPARCstation 5 ever made ran at 170MHz and the 170MHz doesn't even run on Linux as it doesn't support the Fujitsu TurboSPARC (for whatever reason).

To put this in perspective, I tried to today to run Firefox on a multi-core 400MHz UltraSparc-II system (significantly faster clock rate and 3 times as many instructions per cycle).  Technically speaking, it was usable, but 30 seconds to render "google.com" is not encouraging.

Besides which, NativeSparc.cpp already emits V9 instructions. So assuming that the CPU in use is an ultrasparc is completely safe.
Priority: -- → P3
Target Milestone: --- → flash10.1
Is this patch still applicable to newly merged nanojit in tamarin-redux?
Assignee: dennis → leon.sha
OS: Linux → Solaris
Priority: P3 → P4
Hardware: Other → Sun
status anyone?
Status: NEW → ASSIGNED
Priority: P4 → P3
Duplicate of this bug: 502642
Can someone please apply this patch
I'll take the action to land (if appropriate) or close (otherwise), though I do not have access to a sparc-linux system so I don't know how much testing I can really hope to do.

(Leon, feel free to take this back from me if you want.)
Assignee: leon.sha → lhansen
OS: Solaris → Linux
Summary: [patch] sparc linux lacks sync_instruction_memory rendering xulrunner unbuildable → sparc linux lacks sync_instruction_memory rendering xulrunner unbuildable
Whiteboard: Has patch
pushed: redux changeset:   2929:a27726496703
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Engineering work item.  Marking as verified.
Status: RESOLVED → VERIFIED
This is needed on mozilla-1.9.2. Can we get this backported, please?

Thanks
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee: lhansen → nobody
No longer a tamarin issue but a Mozilla nanojit issue.
Priority: P3 → --
Target Milestone: flash10.1 → ---
Flags: flashplayer-qrb+
Whiteboard: Has patch → Has patch, fixed-in-tamarin
Target Milestone: --- → flash10.1
Assignee: nobody → dennis
Component: JIT Compiler (NanoJIT) → Nanojit
Product: Tamarin → Core
QA Contact: nanojit → nanojit
Attachment #386822 - Flags: approval1.9.2.2?
reed, this is not part of the build. We don't ship on solaris. We should just land this imo.
(In reply to comment #23)
> reed, this is not part of the build. We don't ship on solaris. We should just
> land this imo.

just a nit: this is not a patch for solaris, but for linux on sparc processors.
(In reply to comment #23)
> reed, this is not part of the build. We don't ship on solaris. We should just
> land this imo.

In general, approval is still required for landing on branches.
Rules and reality don't have to be mutually exclusive. We don't ship on linux (vendors do), even less so do we ship sparc, and the total number of linux sparc firefox users is probably between 2 and 5. But I guess it can't hurt to get approval (that's why I didn't touch your flag), so lets just wait until one of the drivers wanders by.
(In reply to comment #26)
> We don't ship on linux (vendors do)

We most definitely ship on Linux. I only use mozilla.org builds, for one example.
Attachment #386822 - Flags: approval1.9.2.2? → approval1.9.2.3?
Comment on attachment 386822 [details] [diff] [review]
patch to add sync_instruction_memory

We missed 1.9.2.2.  Moving approval request forward.
Attached patch Patch for 1.9.2Splinter Review
The code moved in 1.9.2, and is not in Assembler.cpp anymore. If that is going to land on 1.9.2, this is the patch.
Attachment #435828 - Flags: approval1.9.2.3?
Attachment #386822 - Flags: approval1.9.2.4?
Attachment #435828 - Flags: approval1.9.2.4? → approval1.9.2.8?
Any news?

C'mon...its an easy fix
Duplicate of this bug: 534608
Another one bites the dust!
Comment on attachment 435828 [details] [diff] [review]
Patch for 1.9.2

Let's try again for next round
Attachment #435828 - Flags: approval1.9.2.9? → approval1.9.2.12?
Comment on attachment 435828 [details] [diff] [review]
Patch for 1.9.2

a=LegNeato for 1.9.2.12. Please land only on the mozilla-1.9.2 default branch, *not* the relbranch.
Attachment #435828 - Flags: approval1.9.2.12? → approval1.9.2.12+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7d8873ec9851
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.