sparc linux lacks sync_instruction_memory rendering xulrunner unbuildable

RESOLVED FIXED in flash10.1

Status

Core Graveyard
Nanojit
--
major
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: Dennis Gilmore, Assigned: Dennis Gilmore)

Tracking

unspecified
flash10.1
Sun
Linux
Bug Flags:
flashplayer-qrb +

Firefox Tracking Flags

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

Details

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

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
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)

Comment 1

8 years ago
Created attachment 386822 [details] [diff] [review]
patch to add sync_instruction_memory
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...

Updated

8 years ago
Attachment #386822 - Flags: review?(gal) → review+

Comment 3

8 years ago
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)

Updated

8 years ago
Blocks: 502642

Comment 4

8 years ago
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+

Updated

8 years ago
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.

Comment 7

8 years ago
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.

Comment 8

8 years ago
(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.

Comment 9

8 years ago
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 ?

Comment 11

8 years ago
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?

Comment 13

8 years ago
@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.

Updated

8 years ago
Priority: -- → P3
Target Milestone: --- → flash10.1

Comment 14

8 years ago
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

Comment 15

8 years ago
status anyone?
Status: NEW → ASSIGNED
Priority: P4 → P3

Updated

8 years ago
Duplicate of this bug: 502642
(Assignee)

Comment 17

8 years ago
Can someone please apply this patch

Comment 18

8 years ago
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

Comment 19

8 years ago
pushed: redux changeset:   2929:a27726496703
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 20

8 years ago
Engineering work item.  Marking as verified.
Status: RESOLVED → VERIFIED

Comment 21

8 years ago
This is needed on mozilla-1.9.2. Can we get this backported, please?

Thanks
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Updated

8 years ago

Updated

8 years ago
Assignee: lhansen → nobody

Comment 22

8 years ago
No longer a tamarin issue but a Mozilla nanojit issue.
Priority: P3 → --
Target Milestone: flash10.1 → ---

Updated

8 years ago
Flags: flashplayer-qrb+
Whiteboard: Has patch → Has patch, fixed-in-tamarin

Updated

8 years ago
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?

Comment 23

8 years ago
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.

Comment 26

8 years ago
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.
Created attachment 435828 [details] [diff] [review]
Patch for 1.9.2

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?

Comment 30

7 years ago
Any news?

C'mon...its an easy fix

Updated

7 years ago
Duplicate of this bug: 534608

Comment 32

7 years ago
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 34

7 years ago
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
Last Resolved: 8 years ago7 years ago
status1.9.2: --- → .12-fixed
Resolution: --- → FIXED

Updated

7 years ago
status1.9.1: --- → wontfix
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.