565250 - TM: remove "guard(class is XYZ)" guards when possible?

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Sometimes we guard on an object's classword when it's clear from previous
context that the object must (or cannot) have that classword, and so the
guard isn't necessary.

An example from access-binary-trees where the guard can never[*] succeed:

    js_NewInstance1 = calli.sro #js_NewInstance ( cx &js_ObjectClass/*136562080*/ $global0 )
    ...
    ldi9 = ldi.o js_NewInstance1[4]
    ~JSSLOT_CLASS_MASK_BITS = immi -4
    andi3 = andi ldi9, ~JSSLOT_CLASS_MASK_BITS/*-4*/
    clasp = immi 136562688
    guard(class is With) = eqi andi3, clasp/*136562688*/
    xt6: xt guard(class is With) -> pc=0x92c8409 imacpc=(nil) sp+56 rp+4 (GuardID=004)

An example from 3d-cube.js where the guard can never[*] fail:

    js_NewEmptyArray1 = calli.sro #js_NewEmptyArray ( cx proto3/*......*/ )
    ...
    ldi36 = ldi.o js_NewEmptyArray1[4]
    andi34 = andi ldi36, ~JSSLOT_CLASS_MASK_BITS20/*-4*/
    guard(class is Array) = eqi andi34, clasp7/*......*/
    xf29: xf guard(class is Array) -> pc=...... imacpc=(nil) sp+184 rp+8 (GuardID=094)

[*] Actually, whether these guards can be removed depends on what occurs in
the '...' portion.  For example, in the latter case it's possible that
js_NewEmptyArray1 might change its classword js_Array to js_SlowArray, in
which case the guard is necessary.

So I'm wondering if any of these optimizations are possible?  Eg. for the
first example, can object's classword ever change from js_ObjectClass to
js_WithClass?  Alternatively, is there some simple analysis that could tell
us if an object's classword has definitely not changed?

If we know that the '...' part doesn't cause problems, then removing the
guard (actually, avoiding generating it in the first place) in jstracer.cpp
is easy.

Comment 1

[Nicholas Nethercote [inactive]]

Nb: the cases that show up in SS/V8:

    js_NewInstance
    js_NewEmptyArray
    js_NewEmptyArrayWithLength
    js_NewArrayWithSlots
    js_Arguments

There's a decent static number of these, but I don't know how many dynamic occurrences that maps to.

Comment 2

[Nicholas Nethercote [inactive]]

I tried implementing the optimisation optimistically, without doing any checking whether the object had changed.  Not safe, but trace-tests all passed.  Results weren't very good.  The biggest SS improvements (instruction counts courtesy of Cachegrind):

3d-raytrace:
  total         139,076,153     138,382,442     1.005x better
  on-trace       25,729,740      25,630,633     1.004x better
access-binary-trees:
  total          81,796,680      81,194,918     1.007x better
  on-trace       18,746,585      18,189,731     1.031x better

3d-raytrace benefited mostly from improved compile-time, as less LIR was pushed into NJ.  There were a few others that were 1.001x or 1.002x better or worse.  V8 results were equally underwhelming.  

Since an unsafe, overly optimistic version of this idea gained us almost nothering, unless someone has some additional flash of insight I'll put this in the "not worth bothering with" bin.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: overly optimistic patch

FWIW, here's the patch implementing the unsafe optimisation.

Comment 4

[Brendan Eich [:brendan]]

Andreas was wondering if this could become important with other changes, IIRC.

/be

Comment 5

[Nicholas Nethercote [inactive]]

(In reply to comment #4)
> Andreas was wondering if this could become important with other changes, IIRC.

Maybe... but that's true of almost any not-very-effective optimisation :)