nanojit: convert i386 codegen macros to functions



Core Graveyard
8 years ago
4 years ago


(Reporter: njn, Assigned: njn)


Firefox Tracking Flags

(Not tracked)


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


(1 attachment)



8 years ago
Created attachment 437271 [details] [diff] [review]

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

8 years ago
Comment on attachment 437271 [details] [diff] [review]

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

8 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

8 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.

Comment 4

8 years ago
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey

Comment 5

7 years ago
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin

Comment 6

7 years ago
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.