Closed Bug 525815 Opened 12 years ago Closed 8 years ago

nanojit: use the obvious argument ordering for insCall()

Categories

(Core Graveyard :: Nanojit, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: n.nethercote, Unassigned)

References

Details

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.
Depends on: 525437
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
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>().
Depends on: 553962
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]
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()
Whiteboard: [mentor=nnethercote@mozilla.com][see comment 4] → [mentor=nnethercote@mozilla.com]
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.
+Bill Maddox.  Edwin is no longer with Adobe.
I'm fairly certain nnethercote won't mentor this.
Whiteboard: [mentor=nnethercote@mozilla.com]
Product: Core → Core Graveyard
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
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.