nanojit: kill reservations in the PPC backend

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

6.52 KB, patch
Rick Reitmaier
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
Bug 514349 started killing off Reservations.  This needs to continue in the PPC backend.  I can make an attempt at a patch but I can't test it at all, and I'm not even sure if TraceMonkey runs on PPC.  Any suggestions on how best to proceed?
(Assignee)

Updated

8 years ago
Blocks: 515313
(Assignee)

Updated

8 years ago
Depends on: 514349
(Assignee)

Comment 1

8 years ago
Created attachment 399961 [details] [diff] [review]
patch

The patch is completely untested, I haven't even compiled it as I don't have access to a PPC.  Julian, if you get TM running on your PPC can you give it a spin?  Thanks.
Attachment #399961 - Flags: review?(jseward)
(In reply to comment #1)
> Created an attachment (id=399961) [details]

I successfully co'd and built tracemonkey (the repo) on ppc32-linux,
and the patch applies, unsurprisingly.  But the build process won't
build Nanojit/TM at all; I just get an interpreter (which works fine).
If someone can tell me how to make the build system build NJ on this
platform I'll try again.

Linux g5 2.6.25.20-0.5-ppc64 #1 SMP 2009-08-14 01:48:11 +0200 ppc64 ppc64 ppc64 GNU/Linux

I configured jsshell thusly:

CC="gcc -m32" CXX="g++ -m32" AR=ar ../configure --enable-debug --enable-optimize="-g -O"

curiously enough, configuring it as follows produces a build failure:

CC="gcc -m32" CXX="g++ -m32" AR=ar ../configure --enable-debug --enable-optimize="-g -O" --target=powerpc-unknown-linux-gnu

Comment 3

8 years ago
Guard exit handling and patching Branches isn't implemented in the PPC backend. LIR_ov is missing, too. You are not going to get far with the current state of the backend. The work isn't difficult though if you know PPC a bit.
> LIR_ov is missing, too. You are not going to get far with the
> current state of the backend.

Oh, sure.  I wasn't wanting to run the backend, merely compile
it, since that's what Nick asked for.  So how do I get it so
that TM and nanojit are at least compiled?

Comment 5

8 years ago
I assume configure.in makes sure we never try to build the non-functional backend. Try to hack it up. We should file a bug to see if a volunteer wants to get the PPC backend up to snuff. Its probably not worth for us to spend time on it, even though its probably just a couple hours we would have to invest.
(Assignee)

Comment 6

8 years ago
Julian, if you can't get configure working quickly we could just commit the change without checking it.  It's the 4th back-end I've made this change for so there's a good chance it's correct or needs only tiny fixes.

Comment 7

8 years ago
Adobe has test coverage for PPC. Ask them for review (rreitmai).
(Assignee)

Comment 8

8 years ago
Comment on attachment 399961 [details] [diff] [review]
patch

Rick, can you test this patch compiles and runs on Tamarin?
Attachment #399961 - Flags: review?(jseward) → review?(rreitmai)

Comment 9

8 years ago
Comment on attachment 399961 [details] [diff] [review]
patch

Eye-balling the patch it looks good.  

Testing will be another story as we need to get tamarin up to date first since your changes rely on un-applied patches to tamarin.
Attachment #399961 - Flags: review?(rreitmai) → review+
(Assignee)

Comment 10

8 years ago
Created attachment 413014 [details] [diff] [review]
new patch

Rick, can you try the patch now?  I just checked, it applies cleanly to TR.  Thanks.

BTW, I'm marking this as blocking bug 512181 because its patch will conflict a little with this one, and I want to get this one in first.
Attachment #399961 - Attachment is obsolete: true
Attachment #413014 - Flags: review?(rreitmai)
(Assignee)

Updated

8 years ago
Blocks: 512181

Comment 11

8 years ago
changed : findRegFor2 -> findRegFor2b  and isKnown -> isKnowReg

otherwise compiled and passed tamarin test suite for Release and Debug Debugger.
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/projects/nanojit-central/rev/e0a0d3915764

I also tested it on the Tamarin try server and it passed, AFAICT -- the waterfall output isn't the easiest to read.  Rick, please feel free to do follow-up fixes if I've broken anything.
Whiteboard: fixed-in-nanojit
(Assignee)

Comment 13

8 years ago
http://hg.mozilla.org/tracemonkey/rev/ef97a9aafcf3
http://hg.mozilla.org/tracemonkey/rev/fd247e17dcae

BTW, the final patch above didn't have r+ but the one before it did, and the final one was almost the same, I just asked rreitmai for the review because I didn't know how to test on PPC, but then I learnt about the Tamarin try server.
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey

Comment 14

8 years ago
Comment on attachment 413014 [details] [diff] [review]
new patch

Marking r+ post-landing ... sorry not it somehow slipped off my radar.
Attachment #413014 - Flags: review?(rreitmai) → review+
(Assignee)

Comment 15

8 years ago
http://hg.mozilla.org/tamarin-redux/rev/fa8ab3ad54b9
http://hg.mozilla.org/mozilla-central/rev/ef97a9aafcf3
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.