1981188 - Add constant index optimisation to DataView load and store instructions

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

[André Bargull [:anba]]

The constant index optimisation to use Address instead of BaseIndex wasn't implemented in https://phabricator.services.mozilla.com/D74623 (bug 1065894), because at that time Spectre index-masking was always enabled.

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 1: Add constant index optimisation to DataView load and store instructions. r=#spidermonkey-reviewers!

Using Address for constant indices wasn't implemented in D74623, because
Spectre index-masking prevented this optimisation. Now that index-masking
isn't always used, we can add the Address optimisation.

dv.getInt32(0, true) was compiled to:

[Codegen]                                 # LIR=ArrayBufferViewElements
[Codegen] movq       0x30(%rax), %rax
[Codegen]                                 # LIR=Integer64
[Codegen] xorl       %edx, %edx
[Codegen]                                 # LIR=LoadDataViewElement
[Codegen] movl       0x0(%rax,%rdx,1), %ecx

And is now compiled to:

[Codegen]                                 # LIR=ArrayBufferViewElements
[Codegen] movq       0x30(%rax), %rax
[Codegen]                                 # LIR=LoadDataViewElement
[Codegen] movl       0x0(%rax), %ecx

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 2: Use ToAddressOrBaseIndex for more unboxed scalar instructions. r=#spidermonkey-reviewers!

Using ToAddressOrBaseIndex allows to remove some duplicated code.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 3: Use ToAddressOrBaseIndex for atomic instructions. r=iain!

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 4: emitElementPostWriteBarrier is never called with a constant index. r=iain!

emitElementPostWriteBarrier is always called with index being a Register.

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 5: Add ToAddressOrBaseObjectElementIndex. r=iain!

Similar to parts 1-3, add ToAddressOrBaseObjectElementIndex to remove a bit of
duplicated code.

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1981188 - Part 6: Prefer BaseObjectElementIndex for BaseIndex into elements. r=iain!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d1638647d226
https://hg.mozilla.org/integration/autoland/rev/66854ea637c1
Part 1: Add constant index optimisation to DataView load and store instructions. r=spidermonkey-reviewers,iain
https://github.com/mozilla-firefox/firefox/commit/b59af1356b64
https://hg.mozilla.org/integration/autoland/rev/9937df0c679b
Part 2: Use ToAddressOrBaseIndex for more unboxed scalar instructions. r=spidermonkey-reviewers,iain
https://github.com/mozilla-firefox/firefox/commit/32db5e2bcd55
https://hg.mozilla.org/integration/autoland/rev/87a250d0827e
Part 3: Use ToAddressOrBaseIndex for atomic instructions. r=iain
https://github.com/mozilla-firefox/firefox/commit/929f05f8f622
https://hg.mozilla.org/integration/autoland/rev/66130f713fa4
Part 4: emitElementPostWriteBarrier is never called with a constant index. r=iain
https://github.com/mozilla-firefox/firefox/commit/d7867c7117e8
https://hg.mozilla.org/integration/autoland/rev/910458f70ca2
Part 5: Add ToAddressOrBaseObjectElementIndex. r=iain
https://github.com/mozilla-firefox/firefox/commit/5c8eb53f933d
https://hg.mozilla.org/integration/autoland/rev/17a8dc178299
Part 6: Prefer BaseObjectElementIndex for BaseIndex into elements. r=iain

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/92c87594c829
https://hg.mozilla.org/integration/autoland/rev/a3de10c45a3d
Revert "Bug 1981188 - Part 6: Prefer BaseObjectElementIndex for BaseIndex into elements. r=iain" for causing sm bustages at CodeGenerator.cpp

Comment 9

[Cristina Horotan [:chorotan]]

Backed out for causing SM bustages

Backout link

Push with failures

Failure log

Comment 10

[André Bargull [:anba]]

GCC doesn't compile code where a template parameter has the same name as another type definition:

struct S {};

struct U {
  using I = S;

  template <typename I>
  static I f(I i);
};

template <typename I>
I U::f(I i) { return i; }

int g(int i) { return U::f(i); }

Maybe https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87841?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5e265cf1813e
https://hg.mozilla.org/integration/autoland/rev/5acde56d1c3b
Part 1: Add constant index optimisation to DataView load and store instructions. r=spidermonkey-reviewers,iain
https://github.com/mozilla-firefox/firefox/commit/2a716cb069f6
https://hg.mozilla.org/integration/autoland/rev/1504f8fde668
Part 2: Use ToAddressOrBaseIndex for more unboxed scalar instructions. r=spidermonkey-reviewers,iain
https://github.com/mozilla-firefox/firefox/commit/feddf9e8d996
https://hg.mozilla.org/integration/autoland/rev/85c16faa35b5
Part 3: Use ToAddressOrBaseIndex for atomic instructions. r=iain
https://github.com/mozilla-firefox/firefox/commit/b81169db4de4
https://hg.mozilla.org/integration/autoland/rev/1f09e4e1321c
Part 4: emitElementPostWriteBarrier is never called with a constant index. r=iain
https://github.com/mozilla-firefox/firefox/commit/d8f88eed29fd
https://hg.mozilla.org/integration/autoland/rev/4135001f01b8
Part 5: Add ToAddressOrBaseObjectElementIndex. r=iain
https://github.com/mozilla-firefox/firefox/commit/a18c51dbca17
https://hg.mozilla.org/integration/autoland/rev/4518aedf5a98
Part 6: Prefer BaseObjectElementIndex for BaseIndex into elements. r=iain

Comment 12

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/5acde56d1c3b
https://hg.mozilla.org/mozilla-central/rev/1504f8fde668
https://hg.mozilla.org/mozilla-central/rev/85c16faa35b5
https://hg.mozilla.org/mozilla-central/rev/1f09e4e1321c
https://hg.mozilla.org/mozilla-central/rev/4135001f01b8
https://hg.mozilla.org/mozilla-central/rev/4518aedf5a98