1626297 - Consider adding IC support for JSOp::Pos

Status: RESOLVED FIXED

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

Description

[André Bargull [:anba]]

https://phabricator.services.mozilla.com/D68869 reminded me that JSOp::Pos doesn't have any IC support, which can show up when using +str to convert strings to numbers [1].

[1] https://stackoverflow.com/questions/1133770/convert-a-string-to-an-integer#answer-43056963 mentions that +str is slower in Firefox than for example str * 1.

Comment 1

[Tom S [:evilpie]]

We should do this, Jan and I talked about this on Matrix.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 1: Clean-up unary operator function callers. r=jandem!

vm/Interpreter-inl.h:

• IncOperation() and DecOperation() should use a non-mutable handle for their
  val parameter.

vm/Interpreter.cpp:

• It's not necessary to create two mutable handles for the same input value.

jit/BaselineIC.cpp:
jit/IonIC.cpp:

• Only when caling BitNot() and NegOperation() we need to copy the input value.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 2: Add assertions to unary operator ic-code. r=jandem!

tryAttachNumber() can actually assert the result is a number when the input
is a number.

Depends on D69514

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 3: Add IC support for JSOp::Pos. r=jandem!

I've went with LoadInt32Result and LoadDoubleResult to match the existing
LoadObjectResult CacheIR opcode.

The arguments jsop_binary_arith() in IonBuilder::jsop_pos() needed to be
swapped to match the requirements in arithUnaryBinaryCache().

Depends on D69515

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 4: Add direct String-to-Int32 CacheIR opcode. r=jandem!

Directly convert Strings to Int32 to make +int32AsString faster and to be
able to have narrower type information for Ion/Warp. Without this change the
unary String ICs for Int32 results from the next part are actually slower than
the ICs for Double results.

emitGuardAndGetInt32FromString() is based on the existing
emitGuardAndGetNumberFromString() method.

Depends on D69516

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 5: Add CacheIR support for unary operators with string inputs. r=jandem!

• emitGuardAndGetNumberFromString() was changed to use ScratchDoubleScope,
  because FloatReg0 isn't available for Unary ICs. (Lowering doesn't reserve it.)
• unaryArithTrySpecializedOnBaselineInspector() now needs to filter String
  values to avoid bailouts.

Depends on D69517

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 6: Check for string indices in MacroAssembler::convertValueToInt. r=jandem!

Ion doesn't use ICs for bit-not ~ with String inputs, which resulted in
+int32AsString being noticeably faster than ~~int32AsString. Handling
string indices makes ~~int32AsString faster than the IC code, which is what
we'd normally expect.

Drive-by change:

• Reuse SimpleBitOpOperand() for bitnotTrySpecialized().

Depends on D69518

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1626297 - Part 7: Remove MToNumber. r=jandem!

Depends on D69519

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c570e074172
Part 1: Clean-up unary operator function callers. r=jandem
https://hg.mozilla.org/integration/autoland/rev/cd56caa32ce4
Part 2: Add assertions to unary operator ic-code. r=jandem
https://hg.mozilla.org/integration/autoland/rev/a9112d79ed33
Part 3: Add IC support for JSOp::Pos. r=jandem
https://hg.mozilla.org/integration/autoland/rev/7adbc910ea0f
Part 4: Add direct String-to-Int32 CacheIR opcode. r=jandem
https://hg.mozilla.org/integration/autoland/rev/1a9370b327b7
Part 5: Add CacheIR support for unary operators with string inputs. r=jandem
https://hg.mozilla.org/integration/autoland/rev/6d84b6a7a8d1
Part 6: Check for string indices in MacroAssembler::convertValueToInt. r=jandem
https://hg.mozilla.org/integration/autoland/rev/933892439025
Part 7: Remove MToNumber. r=jandem

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1c570e074172
https://hg.mozilla.org/mozilla-central/rev/cd56caa32ce4
https://hg.mozilla.org/mozilla-central/rev/a9112d79ed33
https://hg.mozilla.org/mozilla-central/rev/7adbc910ea0f
https://hg.mozilla.org/mozilla-central/rev/1a9370b327b7
https://hg.mozilla.org/mozilla-central/rev/6d84b6a7a8d1
https://hg.mozilla.org/mozilla-central/rev/933892439025