524640 - nanojit: avoid calling releaseRegisters() twice

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch

asm_leave_trace() calls releaseRegisters().  Shortly after, it calls assignSavedRegs() which again calls releaseRegisters().  The call in assignSavedRegs() can be removed without seeming ill effect, and I'm seeing a ~3ms SS speedup on my (64-bit) Mac.

Comment 1

[Andreas Gal :gal]

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

Nice.

Comment 2

[Andreas Gal :gal]

should we maybe move the call into assignSavedRegs? Or at least assert that registers were released?

Comment 3

[Nicholas Nethercote [inactive]]

I'm not sure if calling releaseRegisters() before nFragExit() is important.  I'm happy to add an assertion in assigneSavedRegs() that all the registers are free.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: patch v2

Having removed the call to releaseRegisters() from assignSavedRegs(), better add calls to releaseRegisters() to the other calls sites in the back-ends.

Another possibility would have been to keep the releaseRegisters() call in assignSavedRegs() and remove the one in asm_leave_trace(), but I didn't like that because then the function name would be misleading -- it should really be called releaseRegistersAndAssignSavedRegs().

Comment 5

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/5af925a7a5ce

Comment 6

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/9a76a5c713ee

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/9a76a5c713ee