nanojit: split isQuad() into isI64() + isF64() + is64()



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)


(3 attachments)



8 years ago
Or names something like that.

Comment 1

8 years ago
If we are copying a quad, we should not inhibit register trading (gprs are usually all full, xmms mostly empty).

Comment 2

8 years ago
Sure.  But there may be cases where we put a constant in GpRegs when it's really a float, or in XmmRegs when it's really an int, and then subsequently do arithmetic on it.  These ones will be improved.

Comment 3

8 years ago
Created attachment 422692 [details] [diff] [review]
NJ patch

This patch removes isQuad().  Some details:

- I removed fpn(), because it's identical to gpn() and was only used in a
  couple of places.

- You could argue that I should have kept isQuad() around to use in places
  where we now have 'isI64() || isF64()'.  I deliberately didn't do this
  because I'm trying to remove all traces of the "quad" terminology and the
  type-unsafe thinking behind it.  (If people really want something like
  this, I could add 'isN64()'.)

- I updated SanityFilter to allow testing, but it will be replaced wholesale
  when the LIR type-checker (bug 463137) is committed.
Attachment #422692 - Flags: review?(dvander)


8 years ago
Attachment #422692 - Flags: review?(stejohns)

Comment 4

8 years ago
Created attachment 422693 [details] [diff] [review]
TM patch
Attachment #422693 - Flags: review?(dvander)

Comment 5

8 years ago
Created attachment 422694 [details] [diff] [review]
TR patch

There are a couple of places in this patch where we now have 'ins->isI64()
|| ins->isF64()', but a stronger test of just 'ins->isI64()' or
'ins->isF64()' might be possible, I couldn't tell.  Whoever commits the TR
patch might like to consider this.

Nb: I updated ValidateWriter but the LIR type-checker (bug 463137) should be
committed first, whereupon those changes should be discarded because
ValidateWriter has been moved into nanojit/LIR.cpp.

Passes TR acceptance tests on my local machine, and passes the TR try server.
Attachment #422694 - Flags: review?(stejohns)
Attachment #422692 - Flags: review?(dvander) → review+
Attachment #422693 - Flags: review?(dvander) → review+


8 years ago
Attachment #422692 - Flags: review?(stejohns) → review+


8 years ago
Attachment #422694 - Flags: review?(stejohns) → review+

Comment 6

8 years ago
Whiteboard: fixed-in-nanojit

Comment 7

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


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

Comment 8

8 years ago

Comment 9

8 years ago
Last Resolved: 8 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.