nanojit: convert i386 codegen macros to functions

RESOLVED FIXED

Status

Core Graveyard
Nanojit
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 437271 [details] [diff] [review]
patch

This is good in general -- better type-checking, no danger of forgetting to over-parenthesise arguments.  And it'll also help particularly with bug 506693, where having the extra type-checking on some of the arguments will avoid potential errors because the args are cast before use.
Attachment #437271 - Flags: review?(edwsmith)

Comment 1

7 years ago
Comment on attachment 437271 [details] [diff] [review]
patch

Generally, great!

should these by typedefs?
#define R Register
#define I32 int32_t

IIRC, the inline keyword isn't required on method declarations, even if the definition is marked inline.  (but doesn't hurt, if you want to keep it for doc purposes)

I didn't review line by line for typos or cut+paste errors, counting on testing.
Attachment #437271 - Flags: review?(edwsmith) → review+

Comment 2

7 years ago
Painful as it may be, shouldn't we institute a REALLY_INLINE functionality as we've been using in TR? Otherwise we'll find that some compile config will helpfully ignore plain old "inline"...

Comment 3

7 years ago
maybe, but I haven't pushed on it, because:

* using out-of-line functions with the 'inline' keyword already avoids one of the problems, namely that the  use of 'inline' (or not) inside a class def wasn't treated uniformly.

* we haven't instituted a style where all methods are outside class defs.  (maybe we should, but imho thats a more important issue than REALLY_INLINE).

* to my knowlege, the performance of nanojit doesn't depend critically on any particular one of these functions being inlined.  if it did, i'd be for using REALLY_INLINE there.  but over-use of REALLY_INLINE is a recipe for bloat and eye strain.  I would want to see experimental data on code size and jit performance before committing to a policy in the code.

within TR, we probably err on the side of overuse of REALLY_INLINE in the -inlines.h files, but at least we're being consistent.
(Assignee)

Comment 4

7 years ago
http://hg.mozilla.org/projects/nanojit-central/rev/4062fae8baf2
http://hg.mozilla.org/projects/nanojit-central/rev/3607404c8ab9

http://hg.mozilla.org/tracemonkey/rev/4bf82590923b
http://hg.mozilla.org/tracemonkey/rev/2113638ab7cf
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey

Comment 5

7 years ago
TR: http://hg.mozilla.org/tamarin-redux/rev/ca547af323ea
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin

Comment 6

7 years ago
http://hg.mozilla.org/mozilla-central/rev/4bf82590923b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.