862103 - IonMonkey: Get to benchmark parity with post bug 804676 IonBuilder

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Right now bug 804676's patches (on the IonMonkey repo) regress octane/kraken by a couple %.  These regressions should be fixed before merging to trunk.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: Use baseline information for comparisons

Use baseline information to choose double specializations for comparisons where possible, in the absence of better type information.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: avoid type barrier VM calls with type object guards

v8-deltablue hits some property writes where not all of the objects on the lhs have the property being written, but those other objects will not actually be seen at the site so will never have the property.  Rather than forcing a VM call here this patch adds a bailout testing the type of the object being written to.  This bailout should lead to an invalidation (via the property write itself happening) shortly afterwards.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: fix definite properties bug

This fixes a performance bug where the definite properties analysis would ignore the last property added to 'this' if 'this' escaped later on in its constructor.  Landing this to IonMonkey even though it would also improve trunk, so I guess this is a thumb on the scale.  It seems to help richards a decent amount.

Comment 4

[Brian Hackett [Laid off!]]

Push with the above three patches.  On my machine these bring the 804676 stuff to parity with trunk on octane, or a little better.

https://hg.mozilla.org/projects/ionmonkey/rev/d746d516bf55

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 737736 [details] [diff] [review]
Use baseline information for comparisons

Review of attachment 737736 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/MIR.cpp
@@ +1505,5 @@
> +    // instruction's type policy to insert fallible unboxes to the appropriate
> +    // input types.
> +
> +    if (!strictEq) {
> +        ICStub::Kind kind = inspector->monomorphicStubKind(pc);

What if we add a CompareICInspector class like SetElemICInspector, then this can be

if (inspector.isDoubleComparison()) {
    compareType_ = Compare_Double;
    return;
}

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> What if we add a CompareICInspector class like SetElemICInspector

Or maybe a method that returns the CompareType... As we are going to rely more on baseline caches it will be good to have a single way to access them I think.

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 737742 [details] [diff] [review]
fix definite properties bug

Review of attachment 737742 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch! That bug must've been there for a while...

Comment 8

[Brian Hackett [Laid off!]]

Attachment: baseline comparison info, round 2

Encapsulate queries about comparisons better in the BaselineInspector class.  This applies on top of the previous patch and goes with the second alternative you proposed.  I am not a fan of the ICInspector as it is overengineered for its current use and going with this mechanism means a second one is needed when a query could be about multiple opcodes (maybeMonomorphicShapeForPropertyOp, expectedResultType).

Comment 9

[Brian Hackett [Laid off!]]

Address review comments:

https://hg.mozilla.org/projects/ionmonkey/rev/844053735e04

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 737738 [details] [diff] [review]
avoid type barrier VM calls with type object guards

Review of attachment 737738 [details] [diff] [review]:
-----------------------------------------------------------------

This looks okay, but I'd prefer to see GuardShape/GuardType instead of GuardShapeOrType.

Comment 11

[Brian Hackett [Laid off!]]

Attachment: use GuardShape and GuardObjectType

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d746d516bf55
https://hg.mozilla.org/mozilla-central/rev/844053735e04

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 740783 [details] [diff] [review]
use GuardShape and GuardObjectType

Review of attachment 740783 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 14

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/659fa167a6bb

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/659fa167a6bb