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

RESOLVED FIXED

Status

Core Graveyard
Nanojit
RESOLVED FIXED
8 years ago
4 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

(3 attachments)

(Assignee)

Description

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).
(Assignee)

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.
(Assignee)

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)
(Assignee)

Updated

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

Comment 4

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

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+

Updated

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

Updated

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

Comment 6

8 years ago
http://hg.mozilla.org/projects/nanojit-central/rev/f24a70adec4c
Whiteboard: fixed-in-nanojit
(Assignee)

Comment 7

8 years ago
http://hg.mozilla.org/tracemonkey/rev/9577be7c0add
http://hg.mozilla.org/tracemonkey/rev/a2e95fbd7178
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey

Updated

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

Comment 8

8 years ago
http://hg.mozilla.org/tamarin-redux/rev/9e55dfb8323c
http://hg.mozilla.org/tamarin-redux/rev/b90fb31b12cc

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/9577be7c0add
Status: ASSIGNED → RESOLVED
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.