515311 - nanojit: kill reservations in the PPC backend

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

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?

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 2

[Julian Seward [: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

[Andreas Gal :gal]

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.

Comment 4

[Julian Seward [:jseward]]

> 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

[Andreas Gal :gal]

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.

Comment 6

[Nicholas Nethercote [inactive]]

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

[Andreas Gal :gal]

Adobe has test coverage for PPC. Ask them for review (rreitmai).

Comment 8

[Nicholas Nethercote [inactive]]

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

Rick, can you test this patch compiles and runs on Tamarin?

Comment 9

[Rick Reitmaier]

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.

Comment 10

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 11

[Rick Reitmaier]

changed : findRegFor2 -> findRegFor2b  and isKnown -> isKnowReg

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

Comment 12

[Nicholas Nethercote [inactive]]

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.

Comment 13

[Nicholas Nethercote [inactive]]

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.

Comment 14

[Rick Reitmaier]

Comment on attachment 413014 [details] [diff] [review]
new patch

Marking r+ post-landing ... sorry not it somehow slipped off my radar.

Comment 15

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tamarin-redux/rev/fa8ab3ad54b9
http://hg.mozilla.org/mozilla-central/rev/ef97a9aafcf3