1765779 - Support scalar replacement of array iterator objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[André Bargull [:anba]]

To reproduce:

• Execute below test case with IONFLAGS=logs
• Run iongraph --final /tmp/ion.json.
• Observe that the iterator object wasn't scalar replaced.

function g() {
  var a = [1, 2];
  var r = 0;
  for (var v of a) r += v;
  return r;
}

function f() {
  with ({});
  let r = 0;
  for (let i = 0; i < 100_000; ++i) {
    r += g();
  }
  print(r);
}

with ({});
for (let i = 0; i < 5; ++i) f();

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 1: Handle GuardToClass in object scalar replacement. r=iain!

GuardToArrayIterator in ArrayIteratorNext blocks scalar replacement of the
array iterator object because GuardToClass isn't handled.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 2: Change guard-to pattern to avoid spurious MBox instructions. r=iain!

var obj; is equivalent to var obj = undefined;. This leads to creating a
phi-node, because obj can be either undefined or the this-value. If we
instead initialise var obj = this;, we can ensure that we see only object-typed
definitions during scalar replacement and it ensures we don't emit spurious
MBox instructions for the phi-node input.

Depends on D144290

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 3: Pass object instruction to IsObjectEscaped. r=iain!

Pass the object instruction in preparation for the next part.

Depends on D144291

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 4: Support phi-nodes for iterator object scalar replacement. r=iain!

The guard pattern:

var obj = this;
if (!IsObject(obj) || (obj = GuardToArrayIterator(obj)) === null) {
  return callFunction(CallArrayIteratorMethodIfWrapped, this,
                      "ArrayIteratorNext");
}

creates a phi-node for obj. We can replace this phi-node when all its inputs
are equal to the iterator object we try to scalar replace.

Depends on D144292

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 5: Support MCompare in object scalar replacement. r=iain!

Scalar replacement for iterator objects requires to support this code:

(obj = GuardToArrayIterator(obj)) === null

Depends on D144293

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1765779 - Part 6: Support MIsObject in object scalar replacement. r=iain!

This is the last remaining instruction which needs to be supported for scalar
replacement of iterator objects.

Depends on D144294

Comment 7

[Nicolas B. Pierron [:nbp]]

Thank you for these patches!

I worked on the array side of it in the past and this got regressed many times since the removal of AWFY micro-benchmarks that were checking performance differences between C-style loops and for-of loops.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b22cda4d2fd
Part 1: Handle GuardToClass in object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/6b96d9037d34
Part 2: Change guard-to pattern to avoid spurious MBox instructions. r=iain
https://hg.mozilla.org/integration/autoland/rev/ff95904bc09e
Part 3: Pass object instruction to IsObjectEscaped. r=iain
https://hg.mozilla.org/integration/autoland/rev/c4a0be613d2d
Part 4: Support phi-nodes for iterator object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/9be298355200
Part 5: Support MCompare in object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/460d0aadbf1e
Part 6: Support MIsObject in object scalar replacement. r=iain

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/0b22cda4d2fd
https://hg.mozilla.org/mozilla-central/rev/6b96d9037d34
https://hg.mozilla.org/mozilla-central/rev/ff95904bc09e
https://hg.mozilla.org/mozilla-central/rev/c4a0be613d2d
https://hg.mozilla.org/mozilla-central/rev/9be298355200
https://hg.mozilla.org/mozilla-central/rev/460d0aadbf1e