1198861 - Improve aliasing information and type barrier handling for unboxed arrays

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

The attached patch fixes a couple performance faults with unboxed arrays on sunspider-crypto-aes:

- Improves handling of aliased loads and stores to match that used for normal arrays.
- Avoids type barriers in cases where the objects being filtered out are similar to those being accepted.  This affects both normal and unboxed arrays but we pay a higher price for unboxed arrays because type barrier code generation in this case is so bad --- we load the unboxed pointer, box it, then unbox it, then check its group.  That needs to get fixed too but is more involved, and when possible it's best if we can just avoid the barrier entirely.

Comment 1

[Jan de Mooij [:jandem]]

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

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

Sorry for the delay.

::: js/src/jit/MIR.cpp
@@ +5101,5 @@
> +                TypeSet::ObjectKey* key = property.maybeTypes()->getObject(i);
> +                if (!key)
> +                    continue;
> +
> +                if (!observed->unknownObject()) {

Could invert this condition and add a continue to get rid of one level of indentation.

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1530e404c8df

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1530e404c8df

Comment 4

[Brian Hackett [Laid off!]]

Attachment: more random things

The last patch didn't really help crypto-aes on ss --- this and the other ss tests are so short running that profiling them is difficult.  I think the new type barrier logic actually made things worse; Ion code perf doesn't matter much in this test (since it's so short) and we ended up doing an additional compilation due to these heuristics.  So this patch removes them, and improves handling of type barriers when reading unboxed ObjectOrNull pointers.  That doesn't help perf on this test either, of course.  This patch also fixes some issues with baseline compilation where we were running out of stubs at some access sites when unboxed arrays were being used.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8657316 [details] [diff] [review]
more random things

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

::: js/src/jit/BaselineIC.cpp
@@ +3980,5 @@
>          if (!proto->isNative())
>              return false;
>          if (proto->as<NativeObject>().lastProperty() != nstub->shape(i + 1))
>              return false;
> +        proto = proto->getProto();

Ugh good catch.

@@ +4026,5 @@
> +        if (!iter->isSetElem_DenseOrUnboxedArrayAdd())
> +            continue;
> +
> +        ICSetElem_DenseOrUnboxedArrayAdd* nstub = iter->toSetElem_DenseOrUnboxedArrayAdd();
> +        if (obj->getGroup(cx) != nstub->group())

getGroup is fallible, can it leave a pending exception on the cx?

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/faaafe8c3d1e

Comment 7

[Guilherme Lima]

This patch is a huge regression on Octane.

Comment 8

[Brian Hackett [Laid off!]]

Backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/124d73f46e52

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d29fef133d8a

Comment 10

[Brian Hackett [Laid off!]]

Type set guards with the new BarrierKind were always bailing out on primitive values.  I fixed this and added some debugging code to make sure the barrier kind is acting correctly.

Comment 11

[Hannes Verschore [:h4writer]]

Seems like a few benchmarks on awfy aren't too happy with this push:
https://arewefastyet.com/

Comment 12

[Brian Hackett [Laid off!]]

Well, octane looked fine on my machine.  I don't think I'm going to try to reland this, the ss tests that improved on my machine didn't improve on awfy.

Backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9705e55f06d