nanojit: use the obvious argument ordering for insCall()

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
9 years ago
5 years ago

People

(Reporter: njn, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
insCall() takes the function arguments in reverse order.  I assume/hope this made some kind of sense at one point, but with the change in bug 525437, there really doesn't seem to be any point in it any more, and it's very confusing.
(Reporter)

Updated

9 years ago
Depends on: 525437
(Reporter)

Comment 1

9 years ago
LIns::arg() and CallInfo::get_sizes() also use the reverse order.

In contrast, the bitmask-encoding in CallInfo::_argtypes uses left-to-right order (ie. earlier args are in higher bits) but when shifting to extract these this naturally extracts them in reverse order.  Because _argtypes is involved, it would be best to fix bug 507089 before this one.
Depends on: 507089
(Reporter)

Comment 2

9 years ago
The _argtypes ordering makes it difficult to create functions for building up signatures that build on each other -- eg. it would be nice to have sig1(), sig2(), sig3(), etc, with sig<N>() building on sig<N-1>().
(Reporter)

Updated

9 years ago
Depends on: 553962
(Reporter)

Comment 3

8 years ago
An easier alternative that would give all the benefits of this to TraceMonkey would be to change Writer::call() (in js/src/tracejit/Writer.h) so that its argument array is in the obvious order;  it would then reverse the arguments array in-place before passing it on to LirWriter::insCall().

With a change like this, the hardest part is making sure you've changed all the call sites correctly.  So the best way to proceed would be to rename Writer::call() as Writer::call2(), and recompile.  The compiler will issue an error on every call site, and then you can reverse the order on each call site, one by one.  Once they're all done, you can rename 'call2' as 'call' again, confident you've got every call site.
Whiteboard: [mentor=nnethercote@mozilla.com][see comment 4]
(Reporter)

Updated

7 years ago
Assignee: nnethercote → nobody
Component: JavaScript Engine → Nanojit
QA Contact: general → nanojit
Summary: TM/nanojit: use the obvious argument ordering for insCall() → nanojit: use the obvious argument ordering for insCall()
(Reporter)

Updated

7 years ago
Whiteboard: [mentor=nnethercote@mozilla.com][see comment 4] → [mentor=nnethercote@mozilla.com]
(Reporter)

Updated

7 years ago
Status: ASSIGNED → NEW
Can this be marked as WONTFIX, considering the removal of TM?
Adobe may still want it for Nanojit. CCing Edwin to decide.

Comment 6

7 years ago
+Bill Maddox.  Edwin is no longer with Adobe.
I'm fairly certain nnethercote won't mentor this.
Whiteboard: [mentor=nnethercote@mozilla.com]
(Assignee)

Updated

5 years ago
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
(Reporter)

Comment 8

5 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.