568486 - SH4 (a.k.a ST40) target support for NanoJIT

Status: RESOLVED FIXED

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect, P2)

Description

[Cédric VINCENT]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.3) Gecko/20100402 Firefox/3.6.3
Build Identifier: nanojit-central rev e6ce59a0, tamarin-redux rev 12e6ea17, tracemonkey rev 54acf298

I verified the SH4 support does not introduce regressions on
Tamarin/ARM and Tamarin/x64. I validated this implementation thanks to
the following test-suites:

  product     | revision | test-suite  | # failures / tests | notes
  ------------+----------+-------------+--------------------+------
  nanojit     | e6ce59a0 | lirasm      |          0 / 22    | [1]
  tamarin     | 12e6ea17 | acceptance  |          0 / 59175 |
  tamarin     | 12e6ea17 | performance |          0 / 188   |
  tracemonkey | 54acf298 | trace-test  |          1 / 573   | [2]
  tracemonkey | 54acf298 | smopt-1.9.2 |         44 / 2873  | [3]

Notes:

  [1] I added a test to check floating-point parameters.

  [2] "check-date-format-tofte" fails also on my x64 workstation.

  [3] I am currently working on these failures.


Reproducible: Always




I added a couple of "(void *)" casts to avoid GCC complaining
about "cast from X to Y increases required alignment of target
type", as you did in bug 526666.

Comment 1

[Cédric VINCENT]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT

Comment 2

[Cédric VINCENT]

Attachment: SH4 configuration/build support for NanoJIT

Comment 3

[Cédric VINCENT]

Attachment: New test to check floating-point parameters (nanojit-central)

Comment 4

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tamarin

Comment 5

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tracemonkey

Comment 6

[Brent Baker]

Comment on attachment 447759 [details] [diff] [review]
SH4 configuration/build support for Tamarin

Any idea why there are more tests in spidermonkey/js1_5/Regress/regress-58116.as that are being expected to fail? Or was this happening on your system prior to the patch?

Comment 7

[Cédric VINCENT]

(In reply to comment #6)
> (From update of attachment 447759 [details] [diff] [review])
> Any idea why there are more tests in
> spidermonkey/js1_5/Regress/regress-58116.as that are being expected to fail? Or
> was this happening on your system prior to the patch?

It actually happens on my x64 workstation with the official TR
sources at revision 12e6ea17a457:

  cedric@gnx2503:tamarin$ ./build-x64/shell/avmshell test/acceptance/spidermonkey/js1_5/Regress/regress-58116.abc 
  STATUS: Compute Daylight savings time correctly regardless of year
  Compute Daylight savings time correctly regardless of year Section 1 of test - 1970-07-1  = -60 FAILED! expected: -120
  Compute Daylight savings time correctly regardless of year Section 2 of test - 1965-07-1  = -60 FAILED! expected: -120

Do you think it could be due to my system clock setting?

  cedric@gnx2503:~$ hwclock -u
  Fri 28 May 2010 10:00:15 AM CEST  -0.674614 seconds

  cedric@gnx2503:~$ hwclock --localtime
  Fri 28 May 2010 08:00:25 AM CEST  -0.658986 seconds

Comment 8

[Cédric VINCENT]

(In reply to comment #7)
> Do you think it could be due to my system clock setting?

I confirm it came from my system clock setting:

  cedric@gnx2503:~/Git/netstrictor$ hwclock -u
  Fri 28 May 2010 08:10:37 AM UTC  -0.205850 seconds

  cedric@gnx2503:~/Git/netstrictor$ hwclock --localtime
  Fri 28 May 2010 08:10:42 AM UTC  -0.533966 seconds

  cedric@gnx2503:tamarin$ ./build-x64/shell/avmshell test/acceptance/spidermonkey/js1_5/Regress/regress-58116.abc
STATUS: Compute Daylight savings time correctly regardless of year
Compute Daylight savings time correctly regardless of year Section 1 of test - 1970-07-1  = 0 PASSED!
Compute Daylight savings time correctly regardless of year Section 2 of test - 1965-07-1  = 0 PASSED!

I will remove this change from the next patch, sorry.

Comment 9

[Cédric VINCENT]

(In reply to comment #8)
> I confirm it came from my system clock setting:

I investigated a little bit and it turns out this problem was due to a
permission denied when reading my /etc/localtime, thus it does not
come from the test-suite at all.

Comment 10

[Cédric VINCENT]

Hello,

Do you mind if I submit you an updated patch for the SH4 support?
Actually I don't know if you have reviewed [internally] the previous
one already.

Regards,
Cédric.


New validation report:

  product     | revision | test-suite  | # failures / tests
  ------------+----------+-------------+--------------------
  nanojit     | ccfc1e56 | lirasm      |          0 / 27
  tamarin     | 54d9cd1e | acceptance  |          0 / 59176
  tamarin     | 54d9cd1e | performance |          ? / 290 (WIP)
  tracemonkey | 044852a3 | trace-test  |          1 / 579
  tracemonkey | 044852a3 | smopt-1.9.2 |         27 / 2886


Shortlog relatively to the previous patch:

  Add *untested* support for stacked parameters in Assembler::asm_param()
  Add initial implementation of Assembler::asm_inc_m32()
  Add *untested* implementation of Assembler::asm_store16i()
  Add *untested* support to load "float" values promoted to "double" (ldf2d)
  Add *untested* support to store "double" values demoted to "float" (std2f)
  Add initial implementation of unsigned 8/16 bits loads
  Add initial support for arithmetic with branch on overflow

  Remove duplicated code in Assembler::asm_branch* methods
  Remove improbable cases in asm_arg_regd() and asm_arg_stackd()
  Remove non-optimal case "constant argument handling" in asm_arg_reg()

  Renaming to keep the naming coherency, see NativeSH4.h for details
  Update regalloc' state handling, see bug entry 513615 for details

Comment 11

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tamarin

Comment 12

[Cédric VINCENT]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT

Comment 13

[Dan Smith]

Targeting for Flash Player 10.2

Comment 14

[Rick Reitmaier]

A quick glance of the Tamarin related config patches (3 of 5) looks good.  Will review the 'big' patch (i.e. nanojit target support) whenever you're ready.   

It looks like you're making good progress on the tracemonkey failures.   It would be extremely handy if we could extract lirasm (and/or tamarin) tests (if possible) from these failures.

Also, setting status to Assigned.

Comment 15

[Cédric VINCENT]

(In reply to comment #14)

> Will review the 'big' patch (i.e. nanojit target support) whenever
> you're ready.

I will go off-line the two weeks after this week-end, either we could
start now or we could postpone this review to the beginning of July --
as you wish.

> It looks like you're making good progress on the tracemonkey failures.   It
> would be extremely handy if we could extract lirasm (and/or tamarin) tests (if
> possible) from these failures.

OK I will try to extract lirasm tests from these failures, however I'm
not sure they are all related to NanoJIT nor to the SH4 platform.

Comment 16

[Rick Reitmaier]

(In reply to comment #15)
> (In reply to comment #14)
> 
> > Will review the 'big' patch (i.e. nanojit target support) whenever
> > you're ready.
> 
> I will go off-line the two weeks after this week-end, either we could
> start now or we could postpone this review to the beginning of July --
> as you wish.
> 

I will also be on vacation until mid-July but I've added comments below and cc'ed Rob W. who will also be looking at the patches.

> > It looks like you're making good progress on the tracemonkey failures.   It
> > would be extremely handy if we could extract lirasm (and/or tamarin) tests (if
> > possible) from these failures.
> 
> OK I will try to extract lirasm tests from these failures, however I'm
> not sure they are all related to NanoJIT nor to the SH4 platform.

This is not a strict requirement, but it would be good to understand how/why these failures are occurring.

Comment 17

[Rick Reitmaier]

Quickly glancing at attachment 450323 [details] [diff] [review], the patch's form is quite good and it looks mostly complete; a few comments:
 
I suspect the change to LIR.cpp was an accidental leak into the patch, otherwise we probably want to tease out any changes that affect more than just the SH4 platforms into a separate patch(es) that will require a broader review audience.

Despite many of the other code generators use of #define , its not a requirement so feel free to replace the defines in CodeGenSH4 with functions if you wish.

There are a few asserts that contain "not yet tested".  Is there a way that we can exercise these code paths prior to landing the patch?

Regarding the TODO; 'use a constant pool manager', may I suggest looking at NativeARM as an example.  Also, as a general practice we typically enter new bugs for each longer lived TODO item and then add those to the 'Depends on:' field of the main bug.  This makes it easy to track all sub-items that will eventually get addressed but are not blockers for landing the main bulk of work.  Again not a requirement, but it's a practice we found useful.

Comment 18

[Cédric VINCENT]

(In reply to comment #16)
> This is not a strict requirement, but it would be good to understand how/why
> these failures are occurring.

Actually those failures are all reproducible on x86_64 but
two. Please, could you give me a reliable list of tests to pass?
I'm currently using:

    perl jsDriver.pl -L spidermonkey-n-1.9.2.tests -L slow-n-1.9.2.tests -k -e smopt


(In reply to comment #17)
> I suspect the change to LIR.cpp was an accidental leak into the patch,
> otherwise we probably want to tease out any changes that affect more than just
> the SH4 platforms into a separate patch(es) that will require a broader review
> audience.

Actually the current implementation of shift operators in Tamarin
relies on an unspecified C++ behavior when the number of bits to
shift out are greater than the overall number of available bits
for a given type.


> Despite many of the other code generators use of #define , its not a
> requirement so feel free to replace the defines in CodeGenSH4 with functions if
> you wish.

OK, I prefer typed parameters too.


> There are a few asserts that contain "not yet tested".  Is there a way that we
> can exercise these code paths prior to landing the patch?

Maybe I should learn the LIRasm dialect. How these code paths are
covered in other back-ends?


> Regarding the TODO; 'use a constant pool manager', may I suggest looking at
> NativeARM as an example. Also, as a general practice we typically enter new
> bugs for each longer lived TODO item and then add those to the 'Depends on:'
> field of the main bug.  This makes it easy to track all sub-items that will
> eventually get addressed but are not blockers for landing the main bulk of
> work.  Again not a requirement, but it's a practice we found useful.

OK, I will submit a dozen of items related to SH4 optimizations soon ;)

Thanks,
Cédric.

Comment 19

[Brent Baker]

(In reply to comment #18)
> (In reply to comment #16)
> > This is not a strict requirement, but it would be good to understand how/why
> > these failures are occurring.
> 
> Actually those failures are all reproducible on x86_64 but
> two. Please, could you give me a reliable list of tests to pass?
> I'm currently using:
> 
>     perl jsDriver.pl -L spidermonkey-n-1.9.2.tests -L slow-n-1.9.2.tests -k -e
> smopt
> 
> 


Nick do you know who would be able to provide information to Cedric about how
tracemonkey acceptance failures are tracked? I was trying to find some
information on the MDC site but it simply had a TODO section for the
known-failures
https://developer.mozilla.org/En/SpiderMonkey/Build_Documentation#Test

Comment 20

[Nicholas Nethercote [inactive]]

Those docs are out of date (sigh), there's a new and better script.  Here's how I run the tests, from within js/src/tests/

    python jstests.py -j 2 $PATH_TO_JS/js

where you should instantiate $PATH_TO_JS appropriately.  All tests pass for me on i386 and X64.

Comment 21

[Christophe Guillon]

(In reply to comment #20)
>
>     python jstests.py -j 2 $PATH_TO_JS/js
> 
> where you should instantiate $PATH_TO_JS appropriately.  All tests pass for me
> on i386 and X64.
Concerning tracemonkey validation,
I confirm that these tests pass fully on i386 and X64, though be warned that tests are still dependent on locale and/or timezone setting as it was mentioned first in http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/6aaa2ff7ac683109.
The timezone problem seems to be assigned already in https://bugzilla.mozilla.org/show_bug.cgi?id=526665
I did not find a referenced bug for the dependency on the locale setting (date output format).

In order to pass the tests I used on Linux (I selected a timezone that worked, there are for sure others, but the daylight saving setting is important):
env LANG= TZ=:America/Chicago  python jstests.py -j 2 $PATH_TO_JS/js

We will consider then that this is our new reference for tracemonkey validation.
It fully passes on i386 and X64, next step is to give the status on our SH4 port (not blocking though for tamarin as it is tracemonkey validation).

Comment 22

[Christophe Guillon]

Attachment: SH4 configuration and build support for Tamarin Argo Branch

Comment 23

[Christophe Guillon]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT Tamarin-Argo branch

Must be applied with 455145: SH4 configuration and build support for Tamarin Argo Branch patch

Comment 24

[Christophe Guillon]

Pushed the two patches to apply over the Tamarin Argo branch (id #455145 and id #455148).

Comment 25

[Christophe Guillon]

Attachment: SH4 configuration and build support for Tamarin Redux branch

This patch makes obsolete the previous build configuration support for Tamarin Redux.

Comment 26

[Christophe Guillon]

Comment on attachment 455159 [details] [diff] [review]
SH4 configuration and build support for Tamarin Redux branch

Added USE_PTHREAD_MUTEX for sh4 build.

Comment 27

[Cédric VINCENT]

(In reply to comment #17)
> I suspect the change to LIR.cpp was an accidental leak into the patch,
> otherwise we probably want to tease out any changes that affect more than just
> the SH4 platforms into a separate patch(es) that will require a broader review
> audience.

I added bug 577449 for that.

Comment 28

[Cédric VINCENT]

Good news about the patch attachment 450323 [details] [diff] [review]; there are only 5 failures
reported by the TraceMonkey's testsuite when I ran it through the
Python driver, and all of them are not directly related to the SH4
backend:

 | testcase                            | reason          |
 |-------------------------------------+-----------------|
 | e4x/Regress/regress-355569.js       | out of memory   |
 | e4x/Regress/regress-458679-01.js    | out of memory   |
 | e4x/Regress/regress-458679-02.js    | out of memory   |
 | js1_5/extensions/regress-342960.js  | out of memory   |
 | js1_8_5/regress/regress-555246-1.js | fail on x64 too |

The four first testcases fail because they need more RAM I have on my
board (256MiB). I double-checked their heap consumption on X64 thanks
to libMemusage:

 |                                    | heap peak |
 | testcase                           |    in MiB |
 |------------------------------------+-----------|
 | e4x/Regress/regress-355569.js      |       274 |
 | e4x/Regress/regress-458679-01.js   |       418 |
 | e4x/Regress/regress-458679-02.js   |       378 |
 | js1_5/extensions/regress-342960.js |       258 |

The last testcase (regress-555246-1) fails on my workstation too (TM
rev. 044852a34c7b).

Note: I'm waiting for the bug 513514 to land in Tamarin's sources
before rebasing/testing the SH4 backend against the latest versions of
NanoJIT, TraceMonkey and Tamarin.

Comment 29

[Cédric VINCENT]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT

Comment 30

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tamarin

Comment 31

[Cédric VINCENT]

I rebased the patches "SH4 target support" and "SH4 configuration/build
support for Tamarin" onto revision e07844ed9e2e of Tamarin. The changes
-- relatively to the previous patches -- are:

    Add the Mozilla MPL license header in CodeGenSH4.h
    Use pthread mutex to workaround spinlock problems on some boards

These patches were tested on Tamarin only, that is, they were *not*
tested on NanoJIT/central nor TraceMonkey since their NanoJIT versions
differ from the Tamarin's one (for instance Bug 513514 is missing).
However, I verified they do not introduce any bug in the ARM and
x86_64 backends.

Finally, I removed the change to LIR.cpp (about shift operators) since
it is now tracked by the Bug 577449.

Comment 32

[Edwin Smith]

Comment on attachment 460491 [details] [diff] [review]
SH4 (a.k.a ST40) target support for NanoJIT

Caveat: I haven't reviewed much of the code generation code; if there are bugs, hopefully they turn up in acceptance testing.  (make sure to try -Ojit, for better jit-code coverage).

Overall:
I would discourage the use of macros in CodeGenSH4.h; yes, there is precedent for it, but it's an anti-pattern that we're moving away from.   A better example to follow would be NativeX64.cpp or Nativei386.cpp, which use C++ functions instead of macros.  Functions are easier to debug, and selectively mark inline if jit-speed performance warrants (which it probably doesn't).

If you agree, then also please consider dropping CodeGenSH4.h, and just putting all the code in NativeSH4.h/NativeSH4.cpp, like the other backends.  I'm only offering this as a suggestion; for the rest of us maintaining nanojit, uniformity is desireable.  If there is significant benefits to the extra file, then keep it.

In Assembler.cpp and CodeAlloc.cpp, and elsewhere, can you add a comment explaining why the extra (void*) cast is required, and for which compiler?  (compiler-specific tweaks need justification, lest they hang around forever even after said compiler is fixed).

Should NJ_ALIGN_STACK be something larger than 4?  It's a question of the platform's ABI; for example with gcc on x86, ESP must be 16-byte aligned, even though the hardware only requires ESP to be 4-byte aligned.  Many ABIs favor passing of doubles by requiring at least 8-byte SP alignment.

tip: LIR_subi x,const could also be optimized to use SH4_add_imm, by using -const.

Comment 33

[Cédric VINCENT]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT

Comment 34

[Cédric VINCENT]

Attachment: New testcase to check floating-point parameters (nanojit-central)

Comment 35

[Cédric VINCENT]

Attachment: New testcase to check floating-point single precision load/store (nanojit-central)

Comment 36

[Cédric VINCENT]

Attachment: New testcase to check floating-point single precision load/store (nanojit-central)

Comment 37

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tamarin

Comment 38

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tracemonkey

Comment 39

[Cédric VINCENT]

(In reply to comment #32)

I updated the patches according to your review; here comes the
relative ChangeLog:

    . New LIRasm test-case to check floating-point single precision load/store
    . Convert codegen macros to methods & move these into NativeSH4.* files
    . Optimize integer subtraction by using "SH4_add_imm -immI" when possible
    . The SH4 ABI specifies "double" parameters have to be 8-byte aligned
    . Activate the support for extended load/store opcodes
    . Fix a TOCTTOU bug in Assembler::asm_branch()

These new patches were successfully tested against Tamarin/redux
rev. e1597917, NanoJIT/central rev. 639cf2aa and TraceMonkey
rev. 8cae6d5c.  Many thanks for your review and your suggestions,
I replied to some of your comments right below.


> make sure to try -Ojit, for better jit-code coverage.

I always run the "acceptance" test-suite in the three modes (-Dinterp,
-Ojit, and the default), however the two following code paths were/are
unexpectedly not covered:

    . the floating-point single precision load/store (std2f, ldf2d).
      I discovered thanks to a new LIRasm test-case (attachment 463133 [details] [diff] [review])
      I forgot to define NJ_EXPANDED_LOADSTORE_SUPPORTED.

    . in Assembler::asm_param() when inst->paramKind() == 0 and
      inst->paramArg() >= NumArgRegs.  I don't know how to write a
      new LIRasm test-case for that, maybe someone could help.


> use C++ functions instead of macros [...] also please consider
> dropping CodeGenSH4.h

Done, I prefer functions instead of macros too.


> In Assembler.cpp and CodeAlloc.cpp, and elsewhere, can you add a
> comment explaining why the extra (void*) cast is required, and for
> which compiler?  (compiler-specific tweaks need justification, lest
> they hang around forever even after said compiler is fixed).

Instructions are 16 bits width and general purpose registers are 32
bits width on SH4, thus GCC (4.3.4) complains about potential
alignment problems on the following code (CodeAlloc.cpp):

    holeStart = (NIns*) ((uintptr_t(holeStart) + sizeof(NIns*)-1) & ~(sizeof(NIns*)-1));
    [...]
    CodeList* b2 = (CodeList*) holeStart;

In this example NIns* is 2-byte aligned and CodeList* must be 4-byte
aligned.

Actually, there are already many workarounds to hide compiler warnings
about alignment problems (bug 562017, bug 526666, ...).  I don't
really like void* casts and the -Wno-cast-align option since they just
hide potential [future] runtime problems.  However, I don't know the
best way to adjust correctly the code above.


> Should NJ_ALIGN_STACK be something larger than 4? [...] Many ABIs
> favor passing of doubles by requiring at least 8-byte SP alignment.

You are right, the SH4 ABI specifies "double" parameters have to be
8-byte aligned.  It looks like either there is no test to check that
or I was very lucky!


> tip: LIR_subi x,const could also be optimized to use SH4_add_imm, by
> using -const.

Done, thanks.

Comment 40

[Nicholas Nethercote [inactive]]

Comment on attachment 463135 [details] [diff] [review]
SH4 configuration/build support for Tracemonkey

I doubt anyone will compile TM on sh4, but the patch seems fine.

Comment 41

[Edwin Smith]

Comment on attachment 463134 [details] [diff] [review]
SH4 configuration/build support for Tamarin

Small nit: avmfeatures.as should define VMCFG_SH4, rather than (legacy) AVMPLUS_SH4, since it is new code.  any code in tamarin that needs to check for SH4 should use VMCFG_SH4.  In nanojit, the proper symbol to test for is NANOJIT_SH4 (only spotted one wrong check, In CodeAlloc.cpp:311).  (no need to re-review for this fix).

Comment 42

[Edwin Smith]

Comment on attachment 463130 [details] [diff] [review]
SH4 (a.k.a ST40) target support for NanoJIT

I only skimmed this since I reviewed the earlier patch.

In comment #39 you wrote

    . in Assembler::asm_param() when inst->paramKind() == 0 and
      inst->paramArg() >= NumArgRegs.  I don't know how to write a
      new LIRasm test-case for that, maybe someone could help.

Maybe create a followon bug?  You would need a function written in lirasm syntax that took N non-fp parameters, with N > NumArgRegs, so that at least one parameter would have to be loaded from the stack.

The way tamarin generates jit code, the most incoming parameters we'll need is 3.  If I'm not mistaken, the max # for tracemonkey is 2.  So this is a low priority thing to fix.  The main thing is for the backend to fail clearly and loudly when any limits like this are exceeded, which is what NanoAssert is for.

Comment 43

[Rick Reitmaier]

Comment on attachment 463130 [details] [diff] [review]
SH4 (a.k.a ST40) target support for NanoJIT

Nicely done!  

Also skimming the patch…only encountered one nit in asm_inc_m32() : verbose_only() is probably best reserved for a small 1 or 2 line swath of code, for anything longer #ifdef NJ_VERBOSE …. #endif is the better choice.

Comment 44

[Cédric VINCENT]

Attachment: SH4 (a.k.a ST40) target support for NanoJIT

Comment 45

[Cédric VINCENT]

Attachment: SH4 configuration/build support for Tamarin

Comment 46

[Cédric VINCENT]

Attachment: SH4 configuration/build support for NanoJIT

Comment 47

[Cédric VINCENT]

Attachment: New testcase to check non-FP parameters passed on the stack (nanojit-central)

Comment 48

[Cédric VINCENT]

> Small nit: avmfeatures.as should define VMCFG_SH4, rather than
> (legacy) AVMPLUS_SH4, since it is new code.

> Also skimming the patch…only encountered one nit in asm_inc_m32() :
> verbose_only() is probably best reserved for a small 1 or 2 line
> swath of code, for anything longer #ifdef NJ_VERBOSE …. #endif is
> the better choice.

OK, done.


> You would need a function written in lirasm syntax that took N
> non-fp parameters, with N > NumArgRegs, so that at least one
> parameter would have to be loaded from the stack.

Thanks, I added a new test-case (attachement 468254) in the LIRasm
test-suite for that.


I rebased the patches on top of Tamarin/redux rev. 185bd5e2 and
NanoJIT/central rev. eceee952, the different test-suites didn't report
any trouble. The ChangeLog relatively to the previous version is:

    . Some cosmetic changes
    . Validation of non-FP parameters loaded from the stack
    . Add initial implementation of the opcode LIR_cmovd

Regards,
Cedric.

PS: It takes me too much time to resolve conflicts, re-validate, code
    new features in between two new submissions for three different
    versions of NanoJIT (central, Tamarin & TM), that's why I will
    provide the new patch for TraceMonkey only once the SH4 backend
    has landed into NJ/central and Tamarin/redux.

Comment 49

[Brent Baker]

Using sh4 compilers from this location, cross compiling on ubuntu x86:
http://impactlinux.com/fwl/downloads/binaries/cross-compiler-sh4.tar.bz2

Version information:
Invoked as /home/build/tools/cross-compiler-sh4/bin/sh4-g++
Reference path: /home/build/tools/cross-compiler-sh4/bin/..
arg[ 0] = raw++
arg[ 1] = -fno-use-cxa-atexit
arg[ 2] = -U__nptl__
arg[ 3] = -v
Using built-in specs.
Target: sh-superh-linux
Configured with: /home/landley/temp/firmware/build/temp-sh4/gcc-core/configure --target=sh-superh-linux --prefix=/home/landley/temp/firmware/build/cross-compiler-sh4 --disable-multilib --disable-nls --enable-c99 --enable-long-long --enable-__cxa_atexit --enable-languages=c,c++ --disable-libstdcxx-pch --program-prefix=sh4- --enable-threads=posix --enable-shared --build=x86_64-walrus-linux --host=sh-superh-linux --enable-sjlj-exceptions
Thread model: posix
gcc version 4.2.1


When I apply the tamarin and nanojit patches and attempt to compile with:
../configure.py --enable-shell --target=sh4-linux

I end up with the following error in VMPI/MMgcPortUnix:

Compiling VMPI/MMgcPortUnix
/home/build/hg/tamarin-redux/VMPI/MMgcPortUnix.cpp: In function 'uintptr_t VMPI_getThreadStackBase()':
/home/build/hg/tamarin-redux/VMPI/MMgcPortUnix.cpp:411: error: 'pthread_getattr_np' was not declared in this scope
make: *** [VMPI/MMgcPortUnix.o] Error 1

Have other tamarin folks successfully compiled with these patches? I am sure that I must just have something slightly wrong.

Comment 50

[Cédric VINCENT]

(In reply to comment #49)
> Using sh4 compilers from this location, cross compiling on ubuntu x86:
> http://impactlinux.com/fwl/downloads/binaries/cross-compiler-sh4.tar.bz2
> [...]
> Have other tamarin folks successfully compiled with these patches? I am sure
> that I must just have something slightly wrong.

Maybe you should try with one of these tool-chains (get the packages
named stlinux-XX-cross-...):

    http://ftp.stlinux.com/pub/stlinux/2.4/STLinux/sh4/

    http://ftp.stlinux.com/pub/stlinux/2.3/STLinux/sh4/

These tool-chains are provided by one of the official GCC/SH4
maintainer.

    Configured with: ../configure --host=sh4-linux --build=i686-pc-linux-gnu --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --target=sh4-linux --enable-target-optspace --enable-languages=c,c++ --enable-threads=posix --enable-nls --enable-c99 --enable-long-long --with-system-zlib --enable-shared --disable-libgomp --disable-sjlj-exceptions --enable-multilib --with-multilib-list=m4,m4-nofpu --enable-symvers=gnu --enable-__cxa_atexit
Thread model: posix
gcc version 4.2.4 (snapshot) (STMicroelectronics/Linux Base 4.2.4-57)

Comment 51

[Rick Reitmaier]

pushed nanojit-central changes : http://hg.mozilla.org/projects/nanojit-central/rev/00cee92849b4

Comment 52

[Rick Reitmaier]

pushed lirasm tests (attachment 463131 [details] [diff] [review], 463133, 468254) 
http://hg.mozilla.org/projects/nanojit-central/rev/2c3056daeba3

Comment 53

[Rick Reitmaier]

pushed tamarin changes 
http://hg.mozilla.org/tamarin-redux/rev/f0104adcc42f

Comment 54

[Brent Baker]

(In reply to comment #51)
> pushed nanojit-central changes :
> http://hg.mozilla.org/projects/nanojit-central/rev/00cee92849b4

This patch does not compile cleanly because of recent changes to nanojit::Assembler::asm_spill() in nj rev# 1386

http://hg.mozilla.org/projects/nanojit-central/diff/04a3292575e6/nanojit/Assembler.h

/home/build/buildbot/tamarin-redux/linux-sh4/repo/nanojit/NativeSH4.cpp:2664: error: prototype for ‘void nanojit::Assembler::asm_spill(nanojit::Register, int, bool, bool)’ does not match any in class ‘nanojit::Assembler’
/home/build/buildbot/tamarin-redux/linux-sh4/repo/nanojit/Assembler.h:436: error: candidate is: void nanojit::Assembler::asm_spill(nanojit::Register, int, bool)

Comment 55

[Cédric VINCENT]

Attachment: Synchronize the SH4 backend with Bug 587916

Comment 56

[Brent Baker]

(In reply to comment #55)
> Created attachment 472977 [details] [diff] [review]
> Synchronize the SH4 backend with Bug 587916

Tested out the latest patch in tamarin-redux and the SH4 backend compiles cleanly and passes acceptance.

Comment 57

[Rick Reitmaier]

pushed attachment 472977 [details] [diff] [review] http://hg.mozilla.org/projects/nanojit-central/rev/e5186c20dda9

Comment 58

[Rick Reitmaier]

http://hg.mozilla.org/tamarin-redux/rev/2cffe86fa7f1
http://hg.mozilla.org/tamarin-redux/rev/ef571e0eab52

Comment 59

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/f846d801bfb7

Comment 60

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/75e337029ae0

Comment 61

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/75e337029ae0

Comment 62

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: backport changes from generated cpp to origin selftest

Comment 63

[Brent Baker]

Comment on attachment 481198 [details] [diff] [review]
backport changes from generated cpp to origin selftest

r+ rubber stamp, this matches what is currently checked in for generated cpp code

nit: the tab is only 2 spaces, not the normal 4

Comment 64

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 481198 [details] [diff] [review]
backport changes from generated cpp to origin selftest

Pushed TR changeset 5325:351ea0f490b6
  http://hg.mozilla.org/tamarin-redux/rev/351ea0f490b6