604334 - tests/32-bit/many_params.in failed on SPARC

Status: RESOLVED WONTFIX

(Core Graveyard :: Nanojit, defect)

Description

[Ginn Chen]

Currently asm_param only supports less than 6 params.
argRegs[a] bounds exceeded when a is 6.

Comment 1

[Ginn Chen]

Attachment: patch

Comment 2

[Edwin Smith]

Delegating review to Bill who has a keener eye for Sparc than I do.

Comment 3

[William Maddox]

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

It looks like that in NativeSparc.cpp, calls to underrunProtect() are lifted to the asm_foo() routines rather than performed in the emitters for the individual instructions, e.g., the LDSW32.  I think you need one here, or at least need to convince me (and document in a comment) why one is not required in this case.

Comment 4

[Ginn Chen]

Attachment: patch v2

Thanks for the catch.
I'll check other places for underrunprotect and fix them in other patches.

Comment 5

[William Maddox]

Comment on attachment 485239 [details] [diff] [review]
patch v2

Lifting underrunProtect() to contexts in which the actual length of the emitted code is not obvious seems asking for trouble.  On i386, the low-level instruction emitters take care of this, rather than the 'asm_foo' routines.
Perhaps we should consider this approach on Sparc as well.

Comment 6

[Rick Reitmaier]

Agreed.  You'll want to keep underrunProtect close to the location that actually emits the machine code, ideally in the emitters only.

If you need to protect a sequence of instructions (e.g a location that will be patched later), then placing it outside is ok, but be very careful that the code in between can absolutely honour the implied contract.  You'll probably also want to comment on that type of usage.

Comment 7

[Ginn Chen]

http://hg.mozilla.org/projects/nanojit-central/rev/5a600b5166ea

Comment 8

[Ginn Chen]

(In reply to comment #6)
> Agreed.  You'll want to keep underrunProtect close to the location that
> actually emits the machine code, ideally in the emitters only.
> 
> If you need to protect a sequence of instructions (e.g a location that will be
> patched later), then placing it outside is ok, but be very careful that the
> code in between can absolutely honour the implied contract.  You'll probably
> also want to comment on that type of usage.

Filed Bug 608098

Comment 9

[Rick Reitmaier]

http://hg.mozilla.org/tamarin-redux/rev/51f4eaf0a1ed

Comment 10

[Nicholas Nethercote [inactive]]

Nanojit has been dead for several years. Its Bugzilla component has been moved to the graveyard (bug 984276).

I checked all the open bugs. They're all uninteresting, so I'm WONTFIXing them all. Apologies for the bugspam.