512398 - nanojit: kill RegAlloc::used

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch

RegAlloc has fields 'free' and 'active' which track which registers are free and active at particular times.  It also has a field 'used' whose use is unclear.  Judging from the name it's similar to 'active' but it's only used in isValid(), which is a *very* weak sanity check that is called from only one place.  And it's unclear to me that 'used' is updated in all the places it should be.  And it should only be used with debug builds, but the conditional compilation is inconsistent.

Attached patch removes 'used' and 'isValid()' altogether.  If people want to keep 'used', an explanation of what exactly it's tracking and a judgment of whether it's being updated appropriately would be appreciated!

Comment 1

[Edwin Smith]

Comment on attachment 396377 [details] [diff] [review]
patch

The history of that field is that it was used on register-rich cpu's to keep track of which registers we actually do use, so the generated prolog/epilog don't have to spill everything.

the current approach of using a special LIR_param to track, spill, and restore incoming callee-saved registers works without the used mask, but doesn't integrate well with cpu's that have a mask-based save/restore instruction (arm, ppc).

Given that we're not using the field, the dead code should be removed.

if it comes back later, fine, but maybe something different and well documented will come back later, too.

Comment 2

[Nicholas Nethercote [inactive]]

(In reply to comment #1)
> (From update of attachment 396377 [details] [diff] [review])
> The history of that field is that it was used on register-rich cpu's to keep
> track of which registers we actually do use, so the generated prolog/epilog
> don't have to spill everything.

How is that different to 'active'?  Does 'used' record which registers are used anywhere in the block?

Comment 3

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/1fcee7f53af2

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/1fcee7f53af2

Comment 5

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ff4a305aa502