JM: TypeInference: do floating point register allocation

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

7 years ago
JM should do register allocation for floating point variables and slots in a similar way to other values.  The problem up to now has been not knowing which variables/slots are floats; using a register for something which *could* be a float leads to lots of unnecessary syncing when that decision is wrong.

Using type inference (bug 557407), we can identify variables and stack slots that are definitely ints or floats.  With int -> float coercions in the right places (whenever pushing/storing an int or maybe-int to a float slot), we can ensure such values are always floats and keep them in FP registers.
I wrote a FPRegister allocator a while ago; it was rejected due to adding complexity, and I was told that we just would not be doing that. If things have changed since, that would make me very glad.
(Assignee)

Comment 2

7 years ago
I think with inference things should be simpler.  Registers will only be allocated for things which are definitely floats, and there won't be some situation where a slot can have GPRs for its type and payload, and an FP register on top of that for its (possible) float value.  For slots which could be floats but are not known to be (int|float|undefined, say), the existing allocation strategy will be used (no FP register).
(Assignee)

Comment 3

7 years ago
Created attachment 488773 [details] [diff] [review]
WIP

This adds floating point regalloc within basic blocks, also changes push() methods on FrameState to take information about known types.  Times:

function foo() {
  var x = 1.5;
  var y = 2.8;
  for (var i = 0; i < 10000000; i++)
    x += y;
}
foo();

v8          66ms
jsc         60ms
js -j       33ms
js -m (old) 64ms
js -m (new) 36ms
Assignee: general → bhackett1024
(Assignee)

Comment 4

7 years ago
Created attachment 488975 [details] [diff] [review]
WIP 2

This works on SS.  There are regressions in bitops-bitwise-and, bitops-nsieve-bits and crypto-aes which need to get fixed (there are values in these that are maybe-float but almost-always-int, and we'd be better off treating them as unknown than as definitely-float), 8-9ms improvement in other tests.
Attachment #488773 - Attachment is obsolete: true
(Assignee)

Comment 5

7 years ago
Created attachment 489090 [details] [diff] [review]
WIP 3

This works on SS without regressing anything.  Most of the regressions were due to tripping slow paths on bitop opcodes that handled maybe-double but not known-double (these have been fixed).  Running tests individually, my total speedup between this and bugs 604045 and 608750 is 34ms (12%, or 43ms on AWFY).
Attachment #488975 - Attachment is obsolete: true
(Assignee)

Comment 6

7 years ago
Comment on attachment 489090 [details] [diff] [review]
WIP 3

Not looking for detailed implementation feedback (though it's welcome), but I'd like to make sure the changes this makes to the FrameState API look good.  Would like to get this into JM this week to dovetail with bug 609899.
Attachment #489090 - Flags: feedback?(sstangl)
Attachment #489090 - Flags: feedback?(dvander)
(Assignee)

Comment 7

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

This fixes things so jit-tests work with (and without) inference.
Attachment #489090 - Attachment is obsolete: true
Attachment #489524 - Flags: feedback?(sstangl)
Attachment #489524 - Flags: feedback?(dvander)
Attachment #489090 - Flags: feedback?(sstangl)
Attachment #489090 - Flags: feedback?(dvander)
(Assignee)

Comment 9

7 years ago
Created attachment 490979 [details] [diff] [review]
uncopy patch

Fix for FrameState::uncopy on FP frame entries.

http://hg.mozilla.org/projects/jaegermonkey/rev/ff2aa664dcf8
(Assignee)

Comment 10

7 years ago
Created attachment 494766 [details] [diff] [review]
FrameState register coalescing

This unifies register allocation and management within FrameState for normal registers and FP registers.  Previously these were managed separately, leading to too much near-duplicate logic (and lots more near-duplicate logic in bug 609899).  This moves the contents of FPRegisters into Registers, which can now represent masks containing both normal and FP registers, represented by the type AnyRegisterID.

http://hg.mozilla.org/projects/jaegermonkey/rev/b8b23a892c56
Blocks: 619423
No longer blocks: 608741
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Attachment #489524 - Flags: feedback?(sstangl)
You need to log in before you can comment on or make changes to this bug.