Closed Bug 1780010 Opened 2 years ago Closed 2 years ago

Improve generated code for EqualChars and some further clean-ups

Categories

(Core :: JavaScript Engine, enhancement, P1)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
104 Branch
Tracking Flags:
Tracking Status
firefox104 --- fixed

People

(Reporter: anba, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(12 files)

Bug 1780010 - Part 1: Special-case Latin1Char with char comparison in EqualChars. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 2: Replace ArrayEqual with EqualChars. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 3: Use EqualChars to compare Infinity/NaN for TypedArray indices. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 4: Store "Infinity" as string_view in js_strtod. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 5: Use string_view in more places. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 6: Replace sizeof with std::string_view for function strings. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 7: Replace sizeof with std::end(). r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 8: Replace sizeof with enhanced for-loop. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 9: Replace sizeof with std::size(). r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 10: Use initialiser syntax instead of memset to zero allocate. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 11: Use calloc instead of malloc + memset to zero allocate memory. r=jandem!
(deleted), text/x-phabricator-request
Details
Bug 1780010 - Part 12: Don't pass an explicit string length when the compiler can compute it. r=jandem!
(deleted), text/x-phabricator-request
Details

EqualChars generates sub-optimal code when comparing Latin1Char* against char*: https://godbolt.org/z/5oG6hhd9G.

Also perform some additional clean-ups around EqualChars callers. For example:

  • Use std::string_view where applicable.
  • Replace sizeof where applicable.
Assignee: nobody → andrebargull
Version: unspecified → Trunk

Compilers don't emit memcmp when the both inputs aren't exactly the same type.

This makes the code a bit more uniform.

The previous part should make bug 1589591 obsolete, so it seems useful to make
the code more uniform.

Depends on D152040

Part 1 ensures we generate fast code when comparing against constant char*
strings, so we no longer need the more complicated code.

Depends on D152042

std::string_view stores a pointer and its length, so we no longer have to
use two different variables. std::string_view methods are also const_expr,
so the compiler will inline them at compile-time.

Depends on D152043

builtin/Object.cpp:

  • Using string_view avoids the call to strlen when Consume isn't inlined.

jsapi-tests/testCompileNonSyntactic.cpp:
shell/jsoptparse.cpp:
xpconnect/src/XPCWrappedNativeJSOps.cpp:

  • Using string_view avoids the sizeof - 1 pattern.

Depends on D152044

std::char_traits<char>::length is longer than sizeof - 1, but avoids knowing
how the sizeof - 1 pattern works.

Depends on D152045

Depends on D152047

Depends on D152048

Depends on D152049

Noticed while checking for more sizeof uses across the code base.

Depends on D152051

Noticed because the last part changed memset callers.

Depends on D152052

Attachment #9285864 - Attachment description: Bug 1780010 - Part 6: Replace sizeof with char_trais::length where applicable. r=jandem! → Bug 1780010 - Part 6: Replace sizeof with std::string_view for function strings. r=jandem!
Blocks: jsperf
Severity: -- → S3
Priority: -- → P1
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/de03ad94fa30 Part 1: Special-case Latin1Char with char comparison in EqualChars. r=jandem https://hg.mozilla.org/integration/autoland/rev/03deb487524f Part 2: Replace ArrayEqual with EqualChars. r=jandem https://hg.mozilla.org/integration/autoland/rev/caec1c0fd0cf Part 3: Use EqualChars to compare Infinity/NaN for TypedArray indices. r=jandem https://hg.mozilla.org/integration/autoland/rev/4617fbe2c524 Part 4: Store "Infinity" as string_view in js_strtod. r=jandem https://hg.mozilla.org/integration/autoland/rev/253fba58019e Part 5: Use string_view in more places. r=jandem https://hg.mozilla.org/integration/autoland/rev/effe68106173 Part 6: Replace sizeof with std::string_view for function strings. r=jandem https://hg.mozilla.org/integration/autoland/rev/d5a41f935dc1 Part 7: Replace sizeof with std::end(). r=jandem https://hg.mozilla.org/integration/autoland/rev/eb4437561337 Part 8: Replace sizeof with enhanced for-loop. r=jandem https://hg.mozilla.org/integration/autoland/rev/ee6bcfc78607 Part 9: Replace sizeof with std::size(). r=jandem https://hg.mozilla.org/integration/autoland/rev/d66a2324bd75 Part 10: Use initialiser syntax instead of memset to zero allocate. r=jandem https://hg.mozilla.org/integration/autoland/rev/6c88654690bf Part 11: Use calloc instead of malloc + memset to zero allocate memory. r=jandem https://hg.mozilla.org/integration/autoland/rev/18d92d17a23b Part 12: Don't pass an explicit string length when the compiler can compute it. r=jandem
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: