586358 - TM: make imacpc flagged

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Chris Leary [:cdleary] (not checking bugmail)]

No more storing null to the imacpc value on each frame push.

Comment 1

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: Accessors for imacpc.

JM changes will go in after the merge I suppose.

Comment 2

[Brendan Eich [:brendan]]

Stores are at least async, assuming reservation/buffering resources are not filled up -- is a test and branch really cheaper?

/be

Comment 3

[Luke Wagner [:luke]]

(In reply to comment #2)
This is removing a store on the very hot call path (which we want the mjit to inline, hence size matters) at little or no (when !imacpc) extra cost on much colder paths.

Comment 4

[Nicholas Nethercote [inactive]]

-            if (e->pc == pc && e->imacpc == fp->imacpc &&
+            if (e->pc == pc && (e->imacpc == (fp->hasIMacroPC() ? fp->getIMacro
PC() : NULL)) &&

+    if (innermost->imacpc)
+        fp->setIMacroPC(innermost->imacpc);
+    else
+        fp->clearIMacroPC();

IMO, not allowing getIMacroPC() to return NULL and setIMacroPC() to take NULL is a bad idea.  It requires you to jump through hoops like the cases above, but doesn't gain you any safety AFAICT.  Loosening them might also allow you to remove clearIMacroPC().

Comment 5

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/tracemonkey/rev/7e03c169adee

Addresses njn's getter concern with maybe method.

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/7e03c169adee