1659140 - Move string optimisations for MCharCodeAt and MCompare to GVN and handle non-Latin1 characters in LoadStringCharResult

Status: RESOLVED FIXED

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

Description

[André Bargull [:anba]]

• Move IonBuilder::inlineConstantCharCodeAt to MCharCodeAt::foldsTo
• Move IonBuilder::compareTryCharacter into MCompare::foldsTo
• LoadStringCharResult only handles Latin-1 characters, which prevents any kind of optimisations in Warp.

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1659140 - Part 1: Implement constant charCodeAt optimisation through MCharCodeAt::foldsTo. r=jandem!

This optimisation was added in bug 1041781 and still seems useful to have.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1659140 - Part 2: Optimise string comparisons in Warp. r=jandem!

Implement MCompare::tryFoldCharCompare to port the improvements from
bug 813836 and bug 1538690. This wasn't previously handled through foldsTo,
because relational comparison support for strings (bug 1538692) wasn't present
at that point of time.

Depends on D87098

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1659140 - Part 3: Change LoadStringCharResult to an unshared CacheIR op. r=jandem!

Part 4 will add compiler specific changes to these methods.

Depends on D87099

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1659140 - Part 4: Use VM calls for non-Latin1 characters in LoadStringCharResult. r=jandem!

Restricting LoadStringCharResult to Latin-1 characters means Warp won't be
able to optimise accesses when Two-Byte characters are present. Instead slow
generic VM calls were used for string accesses.

Depends on D87100

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baa5e9e7693a
Part 1: Implement constant charCodeAt optimisation through MCharCodeAt::foldsTo. r=jandem
https://hg.mozilla.org/integration/autoland/rev/5652de79848b
Part 2: Optimise string comparisons in Warp. r=jandem
https://hg.mozilla.org/integration/autoland/rev/602ac0c1f5f1
Part 3: Change LoadStringCharResult to an unshared CacheIR op. r=jandem
https://hg.mozilla.org/integration/autoland/rev/a82a8610a2f9
Part 4: Use VM calls for non-Latin1 characters in LoadStringCharResult. r=jandem

Comment 6

[Nicolas B. Pierron [:nbp]]

Random question, is there any GC concerns on the fact that we are potentially manipulating the memory from an Helper thread, when running with foldsTo, while it could be manipulated on the main thread? (such as ensureLinear for example?)

Comment 7

[André Bargull [:anba]]

The same concern was also raised in the review (https://phabricator.services.mozilla.com/D87098#inline-496503 and https://phabricator.services.mozilla.com/D87099#inline-496529), but because all string constants are atoms, GC can't actually interfere: https://searchfox.org/mozilla-central/rev/6cc48251bb97600fdf11a5b4c5f621bfc8606d55/js/src/jit/MIR.cpp#1001

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/baa5e9e7693a
https://hg.mozilla.org/mozilla-central/rev/5652de79848b
https://hg.mozilla.org/mozilla-central/rev/602ac0c1f5f1
https://hg.mozilla.org/mozilla-central/rev/a82a8610a2f9