This reverts commit 9284ad5731.
Reason for revert: breaks blink tests:
https://ci.chromium.org/p/v8/builders/ci/V8-Blink%20Win/16839
Original change's description:
> [turbofan] Avoid raw InferReceiverMaps in JSCallReducer
>
> Instead provide an abstraction that makes it hard to forget
> dealing with unreliable maps.
>
> This also fixes a deopt loop in Function.prototype.bind and
> one in Array.prototype.reduce.
>
> Bug: v8:9137
> Change-Id: If6a51182c8693a62e9fb6d302cec19b4d48e25cb
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578501
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61106}
TBR=jarin@chromium.org,neis@chromium.org
Change-Id: I97e0f47fb82eda76656905a3f7cc494babd92be6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:9137
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1588433
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61118}
Instead provide an abstraction that makes it hard to forget
dealing with unreliable maps.
This also fixes a deopt loop in Function.prototype.bind and
one in Array.prototype.reduce.
Bug: v8:9137
Change-Id: If6a51182c8693a62e9fb6d302cec19b4d48e25cb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578501
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61106}
This enables constant field tracking unconditionally.
TBR=jgruber@chromium.org
Bug: v8:8361
Change-Id: I02f35827d860c3e0f18a3d55cb156c088d48bc94
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1585730
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61055}
This fixes the bounds check for the 'in' operator to handle the negative
index case properly (by using the same machinery as the potentially
out-of-bounds loads/stores use).
Bug: chromium:952586
Change-Id: I2225acae8be7dcedbcde745e8ef202e789085041
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1581179
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60978}
This adds a new flag --modify-field-representation-inplace (enabled by
default), which lets the runtime perform field representation changes
for Smi to Tagged or for HeapObject to Tagged in-place instead of
creating new maps and marking the previous map tree as deprecated.
That means we create (a lot) fewer Maps and DescriptorArrays in the
beginning and also need to self-heal fewer objects later (migrating
off the deprecated maps). In TurboFan we just take the "field owner
dependency" whenever we use the field representation, which is very
similar to what we already do for the field types. That means if we
change the representation of a field that we used in optimized code,
we will simply deoptimize that code and have TurboFan potentially
later optimize it again with the new field representation.
On the Speedometer2/ElmJS-TodoMVC test, this reduces the total execution
time from around 415ms to around 352ms, which corresponds to a **15%**
improvement. The overall Speedometer2 score improves from around 74.1
to around 78.3 (on local runs with content_shell), corresponding to a
**5.6%** improvement here. 🎉
On the CNN desktop browsing story, it seems that we reduce map space
utilization/fragmentation by about 4-5%. But since we allocate a lot
less (fewer Maps and DescriptorArrays) we also significantly change
the GC timing, which heavily influences the results here. So take this
with a grain of salt. 🤷
Note: For Double fields, this doesn't change anything, meaning they
still create new maps and deprecate the previous map trees.
Bug: v8:8749, v8:8865, v8:9114
Change-Id: Ibd70efcb59be982863905663dbfaa89aa5b31e14
Cq-Include-Trybots: luci.chromium.try:linux-rel,win7-rel
Doc: http://bit.ly/v8-in-place-field-representation-changes
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1565891
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60822}
... all of the kind that modifies the accumulator but no other
registers. Also move a few of that kind out of the IGNORED_BYTECODES
list, where they didn't belong.
R=mslekova@chromium.org
Bug: v8:7790
Change-Id: I67189750e5e01fc8a3b6b5117b61a0d21837693a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561320
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60817}
This reverts commit 1416d5a565.
Reason for revert: blocks roll https://chromium-review.googlesource.com/c/chromium/src/+/1564550
Original change's description:
> [map] Support in-place field representation changes.
>
> This adds a new flag --modify-field-representation-inplace (enabled by
> default), which lets the runtime perform field representation changes
> for Smi to Tagged or for HeapObject to Tagged in-place instead of
> creating new maps and marking the previous map tree as deprecated.
>
> That means we create (a lot) fewer Maps and DescriptorArrays in the
> beginning and also need to self-heal fewer objects later (migrating
> off the deprecated maps). In TurboFan we just take the "field owner
> dependency" whenever we use the field representation, which is very
> similar to what we already do for the field types. That means if we
> change the representation of a field that we used in optimized code,
> we will simply deoptimize that code and have TurboFan potentially
> later optimize it again with the new field representation.
>
> On the Speedometer2/ElmJS-TodoMVC test, this reduces the total execution
> time from around 415ms to around 352ms, which corresponds to a **15%**
> improvement. The overall Speedometer2 score improves from around 74.1
> to around 78.3 (on local runs with content_shell), corresponding to a
> **5.6%** improvement here. 🎉
>
> On the CNN desktop browsing story, it seems that we reduce map space
> utilization/fragmentation by about 4-5%. But since we allocate a lot
> less (fewer Maps and DescriptorArrays) we also significantly change
> the GC timing, which heavily influences the results here. So take this
> with a grain of salt. 🤷♂️
>
> Note: For Double fields, this doesn't change anything, meaning they
> still create new maps and deprecate the previous map trees.
>
> Bug: v8:8749, v8:8865, v8:9114
> Change-Id: I694a53f87ae5caeb868fd98a21809b66d4297d35
> Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
> Doc: http://bit.ly/v8-in-place-field-representation-changes
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561132
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60764}
TBR=jarin@chromium.org,neis@chromium.org,ishell@chromium.org,bmeurer@chromium.org,verwaest@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: v8:8749, v8:8865, v8:9114
Change-Id: I666975d08d51bbe7ab4faec9428b9a1f88e9b322
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1564208
Reviewed-by: Michael Hablich <hablich@chromium.org>
Commit-Queue: Michael Hablich <hablich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60807}
The current NumberEqual check ignores -0 when it is stored to
a constant unboxed double field containing 0.
Bug: v8:9113
Change-Id: I7eb59ca8af09ab7317da3c6ce9c9cedad81f6cae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561317
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60771}
This adds a new flag --modify-field-representation-inplace (enabled by
default), which lets the runtime perform field representation changes
for Smi to Tagged or for HeapObject to Tagged in-place instead of
creating new maps and marking the previous map tree as deprecated.
That means we create (a lot) fewer Maps and DescriptorArrays in the
beginning and also need to self-heal fewer objects later (migrating
off the deprecated maps). In TurboFan we just take the "field owner
dependency" whenever we use the field representation, which is very
similar to what we already do for the field types. That means if we
change the representation of a field that we used in optimized code,
we will simply deoptimize that code and have TurboFan potentially
later optimize it again with the new field representation.
On the Speedometer2/ElmJS-TodoMVC test, this reduces the total execution
time from around 415ms to around 352ms, which corresponds to a **15%**
improvement. The overall Speedometer2 score improves from around 74.1
to around 78.3 (on local runs with content_shell), corresponding to a
**5.6%** improvement here. 🎉
On the CNN desktop browsing story, it seems that we reduce map space
utilization/fragmentation by about 4-5%. But since we allocate a lot
less (fewer Maps and DescriptorArrays) we also significantly change
the GC timing, which heavily influences the results here. So take this
with a grain of salt. 🤷♂️
Note: For Double fields, this doesn't change anything, meaning they
still create new maps and deprecate the previous map trees.
Bug: v8:8749, v8:8865, v8:9114
Change-Id: I694a53f87ae5caeb868fd98a21809b66d4297d35
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Doc: http://bit.ly/v8-in-place-field-representation-changes
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561132
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60764}
Drive-by fix: In ProcessFeedbackForGlobalAccess, we had forgotten to
return the feedback when it already existed.
Bug: v8:7790, v8:9094
Change-Id: Ie4be6cef5755bbdd9d8ed472caaa2e32d243893d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1554680
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60705}
- We didn't take stability dependencies on the inferred maps
in case of kUnreliableReceiverMaps.
- We didn't take stability dependencies on the prototype chains.
Bug: v8:9041
Change-Id: I85418dbed219f51e7fb46c59a0cb9cbb9b499bc1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1541107
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60550}
It was missing a control output.
Bug: chromium:946889
Change-Id: I85f203fc6e27a60f0b86e0e2999dd798a5416dfc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1547655
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60549}
In reducers, we should avoid reductions of the form
ReduceWithValue(node, replacement)
return Replace(node)
because such reduction does not kill the original node, so it may
become subject to resurrection from some side table (in the bug
referenced below it was load elimination's side table). Instead,
we should use
ReduceWithValue(node, replacement)
return Replace(replacement)
Bug: chromium:945644
Change-Id: Id210efe0d214a53241392d30b7f0eee8e7515e2a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1545229
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60517}
Process feedback and hints for Lda/StaNamed bytecodes w.r.t. access on
the global proxy. This stores the property cells (or their absence) on
the JSGlobalProxyData.
Bug: v8:7790
Change-Id: Iadedea5494611c1b2ed38b6ce75687e084cc27f9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1499499
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60411}
ReduceArrayIndexOfIncludes didn't account for kUnreliableReceiverMaps.
Will think about a more robust mechanism for this.
Bug: chromium:944062
Change-Id: Ib2bdaf4399225de4413e12c5684f58dfe524a2cd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1532331
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60400}
There were four places where we did essentially the same steps in
order to extract the initial map for inlining a JSCreate operation.
This CL creates a function on NodeProperties for this task.
As a side effect, this fixes a bug in ReduceJSCreateArray, where
has_initial_map could get called when it wasn't permissible to do so.
Notes: For simplicity, in one or two places where we used to get the
target/newtarget constants from the types we now get them from
HeapConstant nodes.
Cosmetic change: rename "receiver_map" to the more accurate
"root_map" in JSNativeContextSpecialization::ExtractReceiverMaps.
Bug: chromium:939316
Change-Id: I8fd9eb50993be3d839ab9b18eeea28184c53eabf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1528435
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60301}
If the branch associated with the condition is kDead, the current
node will be killed anyway, so let us just survive the lowering.
Bug: chromium:935092
Change-Id: If7b39e3b5452d6c9bc5199080eb38725e6c4eab5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1488769
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60143}
Causes flakyness in TSAN runs when flag is written by EnforceFlagImplications
and read by ConcurrentMarking.
BUG=v8:8924
Change-Id: I2b0bf0fbb678e03492d7ed13e48657de9316b700
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1505796
Auto-Submit: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60062}
This piggy-backs off similar support for lite mode, which silently skips
tests that require optimization in lite (and now jitless) modes.
Bug: v8:7777,v8:8778, v8:8885
Change-Id: I666d92685ca71682224028743f02d0cce3723135
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1503758
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60057}
Bytecode flushing can make tests using assertOptimized flaky if the bytecode is
flushed between marking and optimization. It can also be flaky if the feedback vector
is collected before optimization. To prevent this, a new %PrepareForOptimization
runtime-test function is added that hold onto the bytecode strongly until it is
optimized after being explicitly marked for optimization by %OptimizeFunctionOnNextCall.
BUG=v8:8801,v8:8395
Change-Id: Idbd962a3a2044b915903f9c5e92d1789942b5b41
Reviewed-on: https://chromium-review.googlesource.com/c/1463525
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59914}
When calling a known function from optimized code, where the number of
actual arguments does not match the number of expected arguments,
TurboFan has to call indirectly via the arguments adaptor trampoline,
which creates an argument adaptor frame underneath the activation record
for the callee. This is done so that the callee can still get to the
actual arguments, using either
1. the arguments object, or
2. rest parameters (to get to superfluous arguments), or
3. the non-standard Function.arguments accessor (for sloppy mode
functions), or
4. direct eval(), where we don't know whether there's a use of the
arguments object hiding somewhere in the string.
However going through the arguments adaptor trampoline is quite
expensive usually, it seems to be responsible for over 60% of the
call overhead in those cases.
So this adds a fast path for the case of calling strict mode functions
where we have an arguments mismatch, but where we are sure that the
callee cannot observe the actual arguments. We use a bit on the
SharedFunctionInfo to indicate that this is safe, which is controlled
by hints from the Parser which knows whether the callee uses either
arguments object or rest parameters.
In those cases we use a direct call from optimized code, passing the
expected arguments instead of the actual arguments. This improves the
benchmark on the document below by around 60-65%, which is exactly
the overhead of the arguments adaptor trampoline that we save in this
case.
This also adds a runtime flag --fast_calls_with_arguments_mismatches,
which can be used to turn off the new behavior. This might be handy
for checking the performance impact via Finch.
Bug: v8:8895
Change-Id: Idea51dba7ee6cb989e86e0742eaf3516e5afe3c4
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Doc: http://bit.ly/v8-faster-calls-with-arguments-mismatch
Reviewed-on: https://chromium-review.googlesource.com/c/1482735
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59825}
This lets me run tests with --no-turbo-inlining without having to
worry about false positives.
Change-Id: Icf906e631ef5821136f397af141ba8b18334da7e
Reviewed-on: https://chromium-review.googlesource.com/c/1477730
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59707}
... more precisely, do not mess up the exceptional edges.
Bug: chromium:924151
Change-Id: I3541a1c339c07f509519d4ece6d677dd499f181e
Reviewed-on: https://chromium-review.googlesource.com/c/1429860
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59063}
If feedback for call site frequency is 0, then the combined frequency
is still 0, even if the current function invocation count is infinity.
Bug: chromium:919754
Change-Id: I97be096b6b38f934fb13f01b2b22e148c539e1c0
Reviewed-on: https://chromium-review.googlesource.com/c/1404445
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58714}
Also disables --stress-flush-bytecode on some mjsunit tests which fail
when bytecode flushing is stressed due to test invariants.
Bug=v8:8395
Change-Id: If627910214b3c266e7776340ba182829148e8289
Reviewed-on: https://chromium-review.googlesource.com/c/1372071
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58230}
We assert that loops always have effect phis because there must be
a stack check in every loop. However, with generators, the stack check
may end up outside of loop because the dispatch switch is built first
(while the dispatch switch will also keep the loop backedge alive).
The logic for creating effect phis is already in the code, so
removing the dcheck should be fine.
Bug: chromium:913232
Change-Id: Icf4df831e8b47350543c2b82a34bd3af98782a16
Reviewed-on: https://chromium-review.googlesource.com/c/1372065
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58160}
This is purely a renaming change. The ES spec uses the term 'detach'
for the process of removing the backing store of a typed array, while
V8 uses the historical term 'neuter'. Update our internal implementation,
including method names and flag names, to match the spec.
Note that some error messages still use the term 'neuter' since error
messages are asserted by some embedder tests, like layout tests.
R=bmeurer@chromium.org, yangguo@chromium.org, mstarzinger@chromium.org, mlippautz@chromium.org
BUG=chromium:913887
Change-Id: I62f1c3ac9ae67ba01d612a5221afa3d92deae272
Reviewed-on: https://chromium-review.googlesource.com/c/1370036
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58149}
Currently, if we lower to a pure computation that is unreachable because
of some runtime check, we just rename it with DeadValue. This is
problematic if the pure computation gets later eliminated - that allows
the DeadValue node float above the check that makes it dead. As we
conservatively lower DeadValues to debug-break (i.e., crash), we
might induce crash where we should not.
With this CL, whenever we lower an impossible effectful node (i.e., with
Type::None) to a pure node in simplified lowering, we insert an
Unreachable node there (pinned to the effect chain) and mark the
impossible node dead (and make it depend on the Unreachable node).
Bug: chromium:910838
Change-Id: I218991c79b9e283a9dd5beb4d3f0c4664be76cb2
Reviewed-on: https://chromium-review.googlesource.com/c/1365274
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58066}
This CL moves optimization capabilities from typed lowering to typed
optimization. In particular, this allows retyping of Speculative to
number optimizations depending on their input types. This can save type
checks if we know that inputs are already in SafeIntegerRange and uses
are truncating to 32bit integers.
This change recovers the performance lost to 31bit Smis on
Octane/crypto on x64:
32bit nosmis avg 30,984.84 stddev 180.52
31bit smis (w/o patch) avg 29,438.52 stddev 120.30 -4.99%
31bit smis avg 31,274.52 stddev 176.26 +0.93% +6.24%
Change-Id: I86d6e37305262336f4f7bd46aac0d2cbca11e8c1
Bug: v8:8344
Reviewed-on: https://chromium-review.googlesource.com/c/1323729
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57717}
This is a reland of 585b4eef6a without
any changes.
Original change's description:
> [turbofan] Improve NumberMultiply typing rule.
>
> The NumberMultiply typing rule gave up in the presence of NaN inputs,
> but we can still infer useful ranges here and just union the result
> of that with the NaN propagation (similar for MinusZero propagation).
> This way we can still makes sense of these ranges at the uses.
>
> Bug: v8:8015
> Change-Id: Ic4c5e8edc6c68776ff3baca9628ad7de0f8e2a92
> Reviewed-on: https://chromium-review.googlesource.com/c/1261143
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56539}
Tbr: bmeurer@chromium.org
Bug: v8:8015
Change-Id: I32e5c2f439a1186891ca3393ee53a2a766585839
Reviewed-on: https://chromium-review.googlesource.com/c/1345993
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57664}
This reverts commit 585b4eef6a.
Reason for revert: Speculative, crbug 906567.
Original change's description:
> [turbofan] Improve NumberMultiply typing rule.
>
> The NumberMultiply typing rule gave up in the presence of NaN inputs,
> but we can still infer useful ranges here and just union the result
> of that with the NaN propagation (similar for MinusZero propagation).
> This way we can still makes sense of these ranges at the uses.
>
> Bug: v8:8015
> Change-Id: Ic4c5e8edc6c68776ff3baca9628ad7de0f8e2a92
> Reviewed-on: https://chromium-review.googlesource.com/c/1261143
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56539}
TBR=sigurds@chromium.org,bmeurer@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: v8:8015
Change-Id: I3c652bafbbc0e5d1ad4ff288264fd4f4cbf71330
Reviewed-on: https://chromium-review.googlesource.com/c/1340253
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57602}
Fail IsInvalid check if the property cell has been invalidated.
Bug: chromium:905555
Change-Id: Ia0712b97bd6ba628936b74b3893ddb1c229ee686
Reviewed-on: https://chromium-review.googlesource.com/c/1339863
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57597}
This fixes several problems with instanceof and constant field tracking
in the compiler:
- properly bailout on numbers and non-functions at @@hasInstance.
- deopt on changes of @@hasInstance property.
Bug: v8:8361
Change-Id: I4a1cf9e29d72076f2d37a7c703f18cb2fb8f4040
Reviewed-on: https://chromium-review.googlesource.com/c/1322449
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57532}
As opposed to the register.
For subtle reasons, this fixes a deoptimizer bug with handling return
values in lazy deopt. Since the return values can now only overwrite
the accumulator, there is no danger of overwriting a captured object
that might be later used (since there is no "later").
Bug: chromium:902608
Change-Id: I3a7a10bb1c7a6f4303a01d60f80680afcb7bc942
Reviewed-on: https://chromium-review.googlesource.com/c/1325901
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57349}
This test currently takes nearly 10 minutes on the arm64 debug builder.
Bug: v8:7783
Change-Id: I500fc026b01873e666f32062d790eca3f34455b9
Reviewed-on: https://chromium-review.googlesource.com/c/1318495
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57298}
BinaryNumberOpTyper was not monotonic: if one input changes
from Number to Numeric, while the other input stays BigInt,
the result would change from Number to BigInt.
We have some fuzzing tests for monotonicity but unfortunately
they never generated the inputs required for triggering this bug.
We'll look into improving our tests.
Bug: v8:8380
Change-Id: I7320d9ae4b89ad8798bf9e97cc272edba2162a77
Reviewed-on: https://chromium-review.googlesource.com/c/1307418
Commit-Queue: Hai Dang <dhai@google.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57125}
This adds appropriate LoopExit nodes for the JSCallReducer lowerings of
the following higher order Array builtins:
- Array.prototype.every()
- Array.prototype.find()
- Array.prototype.findIndex()
- Array.prototype.some()
Loop peeling allows TurboFan to make loop invariant operations in the
callback passed to the higher order builtin fully redundant, and thus
completely eliminate the loop invariant code from the subsequent loop
iterations. This can have a huge performance impact, depending on what
kind of code runs inside of the callback. For example, on the micro-
benchmarks outlined in http://crbug.com/v8/8273 we go from
forLoop: 364 ms.
every: 443 ms.
some: 432 ms.
find: 522 ms.
findIndex: 437 ms.
to
forLoop: 369 ms.
every: 354 ms.
some: 348 ms.
find: 419 ms.
findIndex: 360 ms.
which is 20% improvement, and essentially brings the Array builtins (the
appropriate ones Array#some() and Array#every() in this case) on par
with the hand-written `for`-loop.
Bug: v8:1956, v8:8273
Change-Id: I9d32736e5402807b4ac79cd5ad15ceacd1945681
Reviewed-on: https://chromium-review.googlesource.com/c/1305935
Reviewed-by: Daniel Clifford <danno@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57110}
This introduces Word64 support for the CheckBounds operator, which now
lowers to either CheckedUint32Bounds or CheckedUint64Bounds after the
representation selection. The right hand side of CheckBounds can now
be any positive safe integer on 64-bit architectures, whereas it remains
Unsigned31 for 32-bit architectures. We only use the extended Word64
support when the right hand side is outside the Unsigned31 range, so
for everything except DataViews this means that the performance should
remain the same. The typing rule for the CheckBounds operator was
updated to reflect this new behavior.
The CheckBounds with a right hand side outside the Unsigned31 range will
pass a new Signed64 feedback kind, which is handled with newly introduced
CheckedFloat64ToInt64 and CheckedTaggedToInt64 operators in representation
selection.
The JSCallReducer lowering for DataView getType()/setType() methods was
updated to not smi-check the [[ByteLength]] and [[ByteOffset]] anymore,
but instead just use the raw uintptr_t values and operate on any value
(for 64-bit architectures these fields can hold any positive safe
integer, for 32-bit architectures it's limited to Unsigned31 range as
before). This means that V8 can now handle huge DataViews fully, without
falling off a performance cliff.
This refactoring even gave us some performance improvements, on a simple
micro-benchmark just exercising different DataView accesses we go from
testDataViewGetUint8: 796 ms.
testDataViewGetUint16: 997 ms.
testDataViewGetInt32: 994 ms.
testDataViewGetFloat64: 997 ms.
to
testDataViewGetUint8: 895 ms.
testDataViewGetUint16: 889 ms.
testDataViewGetInt32: 888 ms.
testDataViewGetFloat64: 890 ms.
meaning we lost around 10% on the single byte case, but gained 10% across
the board for all the other element sizes.
Design-Document: http://bit.ly/turbofan-word64
Bug: chromium:225811, v8:4153, v8:7881, v8:8171, v8:8383
Change-Id: Ic9d1bf152e47802c04dcfd679372e5c85e4abc83
Reviewed-on: https://chromium-review.googlesource.com/c/1303732
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57095}
For NumberMin and NumberMax we don't need to go to Float64 when the
inputs are known to be in SafeInteger range, instead we can go to
Word64 on 64-bit architectures. This is preliminary work for the
huge DataView support, since we'll utilize NumberMax in that case
to clamp the limit for the bounds check.
Bug: v8:8178, v8:8383
Change-Id: I414114229c5c86b92749d30d645cedc641541ae4
Reviewed-on: https://chromium-review.googlesource.com/c/1304535
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57090}
This changes the ReceiverOrOddball feedback on JSStrictEqual to
ReceiverOrNullOrUndefined feedback, which can also safely be
consumed by JSEqual (we cannot generally accept any oddball here
since booleans trigger implicit conversions, unfortunately).
Thus we replace the previously introduced CheckReceiverOrOddball
with CheckReceiverOrNullOrUndefined, and drop CheckOddball, since
we will no longer collect Oddball feedback separately.
TurboFan will then turn a JSEqual[ReceiverOrNullOrUndefined] into
a sequence like this:
```
left = CheckReceiverOrNullOrUndefined(left);
right = CheckReceiverOrNullOrUndefined(right);
result = if ObjectIsUndetectable(left) then
ObjectIsUndetectable(right)
else
ReferenceEqual(left, right);
```
This significantly improves the peak performance of abstract equality
with Receiver, Null or Undefined inputs. On the test case outlined in
http://crbug.com/v8/8356 we go from
naive: 2946 ms.
tenary: 2134 ms.
to
naive: 2230 ms.
tenary: 2250 ms.
which corresponds to a 25% improvement on the abstract equality case.
For regular code this will probably yield more performance, since we
get rid of the JSEqual operator, which might have arbitrary side
effects and thus blocks all kinds of TurboFan optimizations. The
JSStrictEqual case is slightly slower now, since it has to rule out
booleans as well (even though that's not strictly necessary, but
consistency is key here).
This way developers can safely use `a == b` instead of doing a dance
like `a == null ? b == null : a === b` (which is what dart2js does
right now) when both `a` and `b` are known to be Receiver, Null or
Undefined. The abstract equality is not only faster to parse than
the tenary, but also generates a shorter bytecode sequence. In the
test case referenced in http://crbug.com/v8/8356 the bytecode for
`naive` is
```
StackCheck
Ldar a1
TestEqual a0, [0]
JumpIfFalse [5]
LdaSmi [1]
Return
LdaSmi [2]
Return
```
which is 14 bytes, whereas the `tenary` function generates
```
StackCheck
Ldar a0
TestUndetectable
JumpIfFalse [7]
Ldar a1
TestUndetectable
Jump [7]
Ldar a1
TestEqualStrict a0, [0]
JumpIfToBooleanFalse [5]
LdaSmi [1]
Return
LdaSmi [2]
Return
```
which is 24 bytes. So the `naive` version is 40% smaller and requires
fewer bytecode dispatches.
Bug: chromium:898455, v8:8356
Change-Id: If3961b2518b4438700706b3bd6071d546305e233
Reviewed-on: https://chromium-review.googlesource.com/c/1297315
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56948}
This CL introduces proper Oddball and ReceiverOrOddball states for the
CompareOperationFeedback, and updates the StrictEqual IC to collect this
feedback as well. Previously it would not collect Oddball feedback, not
even in the sense of NumberOrOddball, since that's not usable for the
SpeculativeNumberEqual.
The new feedback is handled via newly introduced CheckReceiverOrOddball
and CheckOddball operators in TurboFan, introduced by JSTypedLowering.
Just like with the Receiver feedback, it's enough to check one side and
do a ReferenceEqual afterwards, since strict equal can only yield true
if both sides refer to the same instance.
This improves the benchmark mentioned in http://crbug.com/v8/8356 from
naive: 2950 ms.
tenary: 2456 ms.
to around
naive: 2996 ms.
tenary: 2192 ms.
which corresponds to a roughly 10% improvement in the case for the
tenary pattern, which is currently used by dart2js. In real world
scenarios this will probably help even more, since TurboFan is able
to optimize across the strict equality, i.e. there's no longer a stub
call forcibly spilling all registers that are live across the call.
This new feedback will be used as a basis for the JSEqual support for
ReceiverOrOddball, which will allow dart2js switching to the shorter
a==b form, at the same peak performance.
Bug: v8:8356
Change-Id: Iafbf5d64fcc9312f9e575b54c32c631ce9b572b2
Reviewed-on: https://chromium-review.googlesource.com/c/1297309
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56925}
When InferReceiverMaps doesn't provide us with reliable maps for the
resolution, we can still utilize the information if all the maps that
are found are stable - aka leaf - maps. But in that case we need to
make sure that we add proper dependencies on the stability of these
maps.
Bug: v8:7253
Change-Id: I6f5825583acc3f2575e83a244d55609ac64d04d3
Reviewed-on: https://chromium-review.googlesource.com/c/1288633
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56789}
The NumberMultiply typing rule gave up in the presence of NaN inputs,
but we can still infer useful ranges here and just union the result
of that with the NaN propagation (similar for MinusZero propagation).
This way we can still makes sense of these ranges at the uses.
Bug: v8:8015
Change-Id: Ic4c5e8edc6c68776ff3baca9628ad7de0f8e2a92
Reviewed-on: https://chromium-review.googlesource.com/c/1261143
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56539}
This change introduces new intrinsics used to desugar async functions
in the Parser and the BytecodeGenerator, namely we introduce a new
%_AsyncFunctionEnter intrinsic that constructs the generator object
for the async function (and in the future will also create the outer
promise for the async function). This generator object is internal
and never escapes to user code, plus since async functions don't have
a "prototype" property, we can just a single map here instead of tracking
the prototype/initial_map on every async function. This saves one word
per async function plus one initial_map per async function that was
invoked at least once.
We also introduce two new intrinsics %_AsyncFunctionReject, which
rejects the outer promise with the caught exception, and another
%_AsyncFunctionResolve, which resolves the outer promise with the
right hand side of the `return` statement. These functions also perform
the DevTools part of the job (aka popping from the promise stack and
sending the debug event). This allows us to get rid of the implicit
try-finally from async functions completely; because the finally
block only called to the %AsyncFunctionPromiseRelease builtin, which
was used to inform DevTools.
In essence we now turn an async function like
```js
async function f(x) { return await bar(x); }
```
into something like this (in Parser and BytecodeGenerator respectively):
```
function f(x) {
.generator_object = %_AsyncFunctionEnter(.closure, this);
.promise = %AsyncFunctionCreatePromise();
try {
.tmp = await bar(x);
return %_AsyncFunctionResolve(.promise, .tmp);
} catch (e) {
return %_AsyncFunctionReject(.promise, e);
}
}
```
Overall the bytecode for async functions gets significantly shorter
already (and will get even shorter once we put the outer promise into
the async function generator object). For example the bytecode for a
simple async function
```js
async function f(x) { return await x; }
```
goes from 175 bytes to 110 bytes (a ~38% reduction in size), which
is in particular due to the simplification around the try-finally
removal.
Overall this seems to improve the doxbee-async-es2017-native test by
around 2-3%. On the test case mentioned in v8:8276 we go from
1124ms to 441ms, which corresponds to a 60% reduction in total
execution time!
Tbr: marja@chromium.org
Bug: v8:7253, v8:7522, v8:8276
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id29dc92de7490b387ff697860c900cee44c9a7a4
Reviewed-on: https://chromium-review.googlesource.com/c/1269041
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56502}
This adds support to the escape analysis to allow scalar replacement
of (small) FixedArrays with element accesses where the index is not a
compile time constant. This happens quite often when inlining functions
that operate on variable number of arguments. For example consider this
little piece of code:
```js
function sum(...args) {
let s = 0;
for (let i = 0; i < args.length; ++i) s += args[i];
return s;
}
function sum2(x, y) {
return sum(x, y);
}
```
This example is made up, of course, but it shows the problem. Let's
assume that TurboFan inlines the function `sum` into it's call site
at `sum2`. Now it has to materialize the `args` array with the two
values `x` and `y`, and iterate through these `args` to sum them up.
The escape analysis pass figures out that `args` doesn't escape (aka
doesn't outlive) the optimized code for `sum2` now, but TurboFan still
needs to materialize the elements backing store for `args` since there's
a `LoadElement(args.elements,i)` in the graph now, and `i` is not a
compile time constant.
However the escape analysis has more information than just that. In
particular the escape analysis knows exactly how many elements a non
escaping object has, based on the fact that the allocation must be
local to the function and that we only track objects with known size.
So in the case above when we get to `args[i]` in the escape analysis
the relevant part of the graph looks something like this:
```
elements = LoadField[elements](args)
length = LoadField[length](args)
index = CheckBounds(i, length)
value = LoadElement(elements, index)
```
In particular the contract here is that `LoadElement(elements,index)`
is guaranteed to have an `index` that is within the valid bounds for
the `elements` (there must be a preceeding `CheckBounds` or some other
guard in optimized code before it). And since `elements` is allocated
inside of the optimized code object, the escape analysis also knows
that `elements` has exactly two elements inside (namely the values of
`x` and `y`). So we can use that information and replace the access
with a `Select(index===0,x,y)` operation instead, which allows us to
scalar replace the `elements`, since there's no escaping use anymore
in the graph.
We do this for the case that the number of elements is 2, as described
above, but also for the case where elements length is one. In case
of 0, we know that the `LoadElement` must be in dead code, but we can't
just mark it for deletion from the graph (to make sure it doesn't block
scalar replacement of non-dead code), so we don't handle this for now.
And for one element it's even easier, since the `LoadElement` has to
yield exactly said element.
We could generalize this to handle arbitrary lengths, but since there's
a cost to arbitrary decision trees here, it's unclear when this is still
beneficial. Another possible solution for length > 2 would be to have
special stack allocation for these backing stores and do variable index
accesses to these stack areas. But that's way beyond the scope of this
isolated change.
This change shows a ~2% improvement on the EarleyBoyer benchmark in
JetStream, since it benefits a lot from not having to materialize these
small arguments backing stores.
Drive-by-fix: Fix JSCreateLowering to properly initialize "elements"
with StoreElement instead of StoreField (which violates the invariant
in TurboFan that fields and elements never alias).
Bug: v8:5267, v8:6200
Change-Id: Idd464a15a81e7c9653c48c814b406eb859841428
Reviewed-on: https://chromium-review.googlesource.com/c/1267935
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56442}
This change adds predicates to check whether a given JavaScript operator
needs the "current context" or if any surrounding context (including the
"native context") does it. For example JSAdd doesn't ever need the
current context, but actually only the native context. In the
BytecodeGraphBuilder we use this predicate to check whether a given
operator needs the current context, and if not, we just pass in the
native context.
Doing so we improve the performance on the benchmarks given in the
tracking bug significantly, and go from something around
arrayMap: 476 ms.
arrayFilter: 312 ms.
arrayEvery: 241 ms.
arraySome: 152 ms.
to
arrayMap: 377 ms.
arrayFilter: 296 ms.
arrayEvery: 191 ms.
arraySome: 91 ms.
which is an up to 40% improvement. So for idiomatic modern JavaScript
which uses higher order functions quite a lot, not just the builtins
provided by the JSVM, this is going to improve peak performance
noticably.
This also makes it possible to completely eliminate all the allocations
in the aliased sloppy arguments example
```js
function foo(a) { return arguments.length; }
```
concretely we don't allocate the function context anymore and we also
don't allocate the arguments object anymore (the JSStackCheck was the
reason why we did this in the past, because it was holding on to the
current context, which also kept the allocation for the arguments
alive).
Bug: v8:6200, v8:8060
Change-Id: I1db56d00d6b510ce6337608c0fff16af96e95eef
Design-Document: bit.ly/v8-turbofan-context-sensitive-js-operators
Reviewed-on: https://chromium-review.googlesource.com/c/1267176
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56441}
As identified in the web-tooling-benchmark, there are specific code
patterns involving array indexed property accesses and subsequent
comparisons of those indices that lead to repeated Smi checks in the
optimized code, which in turn leads to high register pressure and
generally bad register allocation. An example of this pattern is
code like this:
```js
function f(a, n) {
const i = a[n];
if (n >= 1) return i;
}
```
The `a[n]` property access introduces a CheckBounds on `n`, which
later lowers to a `CheckedTaggedToInt32[dont-check-minus-zero]`,
however the `n >= 1` comparison has collected `SignedSmall` feedback
and so it introduces a `CheckedTaggedToTaggedSigned` operation. This
second Smi check is redundant and cannot easily be combined with the
earlier tagged->int32 conversion, since that also deals with heap
numbers and even truncates -0 to 0.
So we teach the RedundancyElimination to look at the inputs of these
speculative number comparisons and if there's a leading bounds check
on either of these inputs, we change the input to the result of the
bounds check. This avoids the redundant Smi checks later and generally
allows the SimplifiedLowering to do a significantly better job on the
number comparisons. We only do this in case of SignedSmall feedback
and only for inputs that are not already known to be in UnsignedSmall
range, to avoid doing too many (unnecessary) expensive lookups during
RedundancyElimination.
All of this is safe despite the fact that CheckBounds truncates -0
to 0, since the regular number comparisons in JavaScript identify
0 and -0 (unlike Object.is()). This also adds appropriate tests,
especially for the interesting cases where -0 is used only after
the code was optimized.
Bug: v8:6936, v8:7094
Change-Id: Ie37114fb6192e941ae1a4f0bfe00e9c0a8305c07
Reviewed-on: https://chromium-review.googlesource.com/c/1246181
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56428}
This reverts commit 4fd92b252b.
Reason for revert: Significant tankage on the no-mitigations bots (bad timing on the regular bots)
Original change's description:
> [turbofan] Do not consume SignedSmall feedback in TurboFan anymore.
>
> This changes TurboFan to treat SignedSmall feedback similar to Signed32
> feedback for binary and compare operations, in order to simplify and
> unify the machinery.
>
> This is an experiment. If this turns out to tank performance, we will
> need to revisit and ideally revert this change.
>
> Bug: v8:7094
> Change-Id: I885769c2fe93d8413e59838fbe844650c848c3f1
> Reviewed-on: https://chromium-review.googlesource.com/c/1261442
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56411}
TBR=jarin@chromium.org,bmeurer@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: v8:7094
Change-Id: I9fff3b40e6dc0ceb7611b55e1ca9940089470404
Reviewed-on: https://chromium-review.googlesource.com/c/1267175
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56427}
This changes TurboFan to treat SignedSmall feedback similar to Signed32
feedback for binary and compare operations, in order to simplify and
unify the machinery.
This is an experiment. If this turns out to tank performance, we will
need to revisit and ideally revert this change.
Bug: v8:7094
Change-Id: I885769c2fe93d8413e59838fbe844650c848c3f1
Reviewed-on: https://chromium-review.googlesource.com/c/1261442
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56411}
The CheckSmi in String.fromCodePoint() is unnecessary and even leads to
unnecessary deoptimizations, since the CheckBounds already does the
right thing, plus it also handles HeapNumbers (in Signed32 range) and
properly identifies zeros.
Bug: v8:8238
Change-Id: I73bf7a70c3cd718c987f112ceb928188c0534cd5
Reviewed-on: https://chromium-review.googlesource.com/c/1262675
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56395}
For NumberModulus and SpeculativeNumberModulus there's no observable
difference between 0 and -0 for the right hand side, since both of them
result in NaN (in general the sign of the right hand side is ignored
for modulus in JavaScript). For the left hand side we can just propagate
the zero identification part of the truncation, since we only care about
-0 on the left hand side if the use nodes care about -0 too.
This further improves the Kraken/audio-oscillator test from around 67ms
to 64ms.
Bug: v8:8015, v8:8178
Change-Id: I1f51d42f7df08aaa28a9b0ddd3177df6b76be98c
Reviewed-on: https://chromium-review.googlesource.com/c/1260024
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56372}
This is a follow-up cleanup to treat NumberRound like the other rounding
operations (NumberFloor, NumberCeil and NumberTrunc).
Bug: v8:8015
Change-Id: I2b2fbc7f0319497d16ccb7472595eeb68be1f51d
Reviewed-on: https://chromium-review.googlesource.com/c/1260403
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56371}
The slow-path of CheckedInt32Mod(x,y) when x is found to be negative
still had the power of two right hand side optimization, and thus would
perform a dynamic check on y. Now the same dynamic check was done for
the fast-path, and the word operations for this check were pure, leading
to weird bit materialization in TurboFan (due to sea of nodes). But
there's not really a point to be clever for the slow-path, so we just
insert the Uint32Mod operation directly here, which completely avoids
the problem.
This improves the Kraken/audio-oscillator test from around 73ms to 69ms.
Bug: v8:8069
Change-Id: Ie8ea667136c95df2bd8c5ba56ebbc6bd2442ff23
Reviewed-on: https://chromium-review.googlesource.com/c/1259063
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56370}
Following up on the earlier work regarding redundant Smi checks in
https://chromium-review.googlesource.com/c/v8/v8/+/1246181, it was
noticed that the handling of the 0 and -0 and how some operations
identify these is not really consistent, but was still rather ad-hoc.
This change tries to unify the handling a bit by making sure that all
number comparisons generally pass truncations that identify zeros, since
for the number comparisons in JavaScript there's no difference between
0 and -0. In the same spirit NumberAbs and NumberToBoolean should also
pass these truncations, since they also don't care about the differences
between 0 and -0.
Adjust NumberCeil, NumberFloor, NumberTrunc, NumberMin and NumberMax
to pass along any incoming kIdentifiesZeros truncation, since these
operations also don't really care whether the inputs can be -0 if the
use nodes don't care.
Also utilize the kIdentifiesZeros truncation for NumberModulus with
Signed32 inputs, because it's kind of common to do something like
`x % 2 === 0`, where it doesn't really matter whether `x % 2` would
eventually produce a negative zero (since that would still be considered
true for the sake of the comparison).
This also adds a whole lot of tests to ensure that not only are these
optimizations correct, but also that we do indeed perform them.
Drive-by-fix: The `NumberAbs(x)` would incorrectly lower to just `x` for
PositiveIntegerOrMinusZeroOrNaN inputs, which was obviously wrong in
case of -0. This was fixed as well, and an appropriate test was added.
The reason for the unification is that with the introduction of Word64
for CheckBounds (which is necessary to support large TypedArrays and
DataViews) we can no longer safely pass Word32 truncations for the
interesting cases, since the index might be outside the Signed32 or
Unsigned32 ranges, but we still identify 0 and -0 for the sake of the
bounds check, and so it's important that this is handled consistently
to not regress performance on TypedArrays and DataViews accesses.
Bug: v8:8015, v8:8178
Change-Id: Ia1d32f1b726754cea1e5793105d9423d84a6393a
Reviewed-on: https://chromium-review.googlesource.com/1246172
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56325}
It was shipped in Chrome 67.
Bug: v8:6791, v8:8238
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;luci.v8.try:v8_linux_noi18n_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I94d8f0aa18570452403a35dea270b18f155c970a
Reviewed-on: https://chromium-review.googlesource.com/1253604
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56310}
Currently, we call the MapRef::AsElementsKind method on an initial
map multiple times (from JSCreateLowering::ReduceJSCreateArray).
However, this does not does not play well with the heap copier/broker,
which only expectes AsElementsKind to be called on initial maps.
This CL makes sure we only call AsElementsKind once (on the initial map).
Bug: chromium:890620
Change-Id: If44421d3900abb7629ea8f789a005b8d8ebaf881
Reviewed-on: https://chromium-review.googlesource.com/1253105
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56307}
Bug: chromium:890057
Change-Id: I98bc278ebc202c3d8f6417367bd1c592e4824011
Reviewed-on: https://chromium-review.googlesource.com/1250481
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56279}
Properly test the abstract equality - both JSEqual and JSNotEqual - for
the case of symbols. Also add tests for the corner cases of the
JSObjectIsArray operator, which is used to implement Array.isArray()
builtin.
Bug: v8:8015
Change-Id: Ib008e85553d04527a5992a904ec77774761f872e
Reviewed-on: https://chromium-review.googlesource.com/1238237
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56121}
Improve the lowering of CheckedInt32Div and CheckedUint32Div for the
case that the right hand side is a known (positive) power of two, as
in that case it's sufficient to just check the relevant bits on the
left hand side and then shift by the appropriate amount of bits.
This is significantly faster than what TurboFan is able to generate
from the general lowering, even with all the MachineOperatorReducer
magic (it even shows as a steady ~1.5% overall improvement on the
Kraken crypto ccm benchmark).
Also turn the general CheckedInt32Div lowering into readable code again,
and make sure that all the bailout cases are properly covered by mjsunit
tests (i.e. the "division by zero" bailout was not covered properly).
Bug: v8:8015
Change-Id: Ibfdd367a6ee5d70dcaa48801858042c5029b7004
Reviewed-on: https://chromium-review.googlesource.com/1236954
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56115}
The previous tests didn't cover the case Number.isSafeInteger(x)
where TurboFan was unable to tell that `x` is always a Number and
thus had to use the ObjectIsSafeInteger operator instead.
Bug: v8:8015
Change-Id: I9bdbfa602fe0bf8c5fb2bc6c160ace7ab0bc0aaa
Reviewed-on: https://chromium-review.googlesource.com/1238234
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56114}
Again in the spirit of https://chromium-review.googlesource.com/1226033
we can simplify the handling of NumberDivide and decide the lowering
based on the feedback type.
Drive-by-fix: Add test coverage for the relevant corner cases of the
NumberDivide handling in SimplifiedLowering.
Bug: v8:8015
Change-Id: I0edaca0fddb31d64d2c269268e87a32a687a0b26
Reviewed-on: https://chromium-review.googlesource.com/1236262
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56113}
The ObjectIsArrayBuffer simplified operator, which is used to implement
the ArrayBuffer.isView() builtin, didn't have any test coverage.
Bug: v8:8015
Change-Id: Ia15e35bc4ae61627137f7a89976560a8d3db771f
Reviewed-on: https://chromium-review.googlesource.com/1238215
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56112}
In the spirit of https://chromium-review.googlesource.com/1226033 we can
also unify the handling of NumberModulus based on feedback types.
Drive-by-fix: Add appropriate tests for the corner cases of the
NumberModules with (surrounding) feedback integration.
Bug: v8:8015
Change-Id: I5e3207d2f6e72f9ea1d7658014b7272075088d63
Reviewed-on: https://chromium-review.googlesource.com/1236260
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56094}
The coverage bot figured out that there's missing test coverage
for the SpeculativeNumberModulus corner cases inside of the
SimplifiedLowering logic.
Bug: v8:8015
Change-Id: Id32aa545dc43adae5e67c66574ccea5f2b3db846
Reviewed-on: https://chromium-review.googlesource.com/1236259
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56093}
This adds missing test coverage for corner cases of SpeculativeNumberAdd
and SpeculativeNumberSubtract inside of SimplifiedLowering. This was
discovered to be untested by the coverage bot.
Bug: v8:8015
Change-Id: I7355b1b840a76bc12bd911adb6c2d88f05d816c5
Reviewed-on: https://chromium-review.googlesource.com/1236256
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56090}
Part of https://chromium-review.googlesource.com/1231994 that landed
earlier, but was reverted due to breakage. Landing this cleanup
separately instead.
Drive-by-fix: Also add test coverage for the cases that weren't covered
properly (according to the test coverage bot).
Bug: chromium:225811, v8:8015
Change-Id: I9c13ed5fcf0ba9e6b190489e15df86970eafdc13
Reviewed-on: https://chromium-review.googlesource.com/1236213
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56087}
According to the coverage bot, there's some lack of test coverage for
corner cases of Math.imul(). Add the missing test coverage and also
add some coverage for the generally interesting cases.
Bug: v8:8015
Change-Id: I2a917283b4777510fb5db421a039ff0de9b2a25f
Reviewed-on: https://chromium-review.googlesource.com/1235577
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56077}
Remove the NumberConstant right hand side limitation for the speculative
number operation optimization, and extend the logic to also deal with
SpeculativeToNumber, which is common when dealing with postfix increment
and array operations.
Also add appropriate tests for all the relevant cases, specifically we
mjsunit tests to increase the general coverage for the various cases
here (in addition to dedicated unittests).
Bug: v8:8015
Change-Id: I8c92f98490c63b07eb19686efd404322979e57c4
Reviewed-on: https://chromium-review.googlesource.com/1235919
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56072}
If type checks in simplified lowering produced dead value (i.e., of
type Type::None()), we have only propagated deadness along value
edges. With this CL, we also insert an Unreachable node after every
effectful node that produces dead value.
This is more consistent with dead code elimination, which also inserts
unreachable nodes after effectful nodes with value output None.
Bug: chromium:884052
Change-Id: Idcb168461f05f1811b2c9c16ab8ff179b259fbd3
Reviewed-on: https://chromium-review.googlesource.com/1228125
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55987}
For NumberAdd/Subtract/Multiply we currently onlt consult the upper
bound to decide whether to compute using Int32 or Float64 operations,
whereas for NumberModulus, NumberEqual, etc. we do decide based on
the feedback types, where the only significant difference is that we
cannot promise Word32 truncations on the inputs.
This change unifies the handling for NumberAdd/Subtract/Multiply as
well, which triggers surprisingly often in our core benchmark suites..
Bug: v8:8015
Change-Id: If8ec1bc82d1e1b71285c829262a0d343a4eb2af7
Reviewed-on: https://chromium-review.googlesource.com/1226033
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55943}
This change introduces the necessary conversion operators to convert
from Word64 to other representations (Tagged, Word32, Float64, etc.),
and plugs in the Word64 representation for NumberAdd/NumberSubtract,
such that TurboFan will go to Int64Add/Sub on 64-bit architectures
when the inputs and the output of the operation is in safe integer
range. This includes the necessary changes to the Deoptimizer to be
able to rematerialize Int64 values as Smi/HeapNumber when going back
to Ignition later.
This change might affect performance, although measurements indicate
that there should be no noticable performance impact.
The goal is to have TurboFan support Word64 representation to a degree
that changing the TypedArray length to an uint64_t (for 64-bit archs)
becomes viable and doesn't have any negative performance implications.
Independent of that we might get performance improvements in other areas
such as for crypto code later.
Bug: v8:4153, v8:7881, v8:8171, v8:8178
Design-Document: bit.ly/turbofan-word64
Change-Id: I29d56e2a31c1bae61d04a89d29ea73f21fd49c59
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel
Reviewed-on: https://chromium-review.googlesource.com/1225709
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55937}
The typing rules for NumberMax and NumberMin didn't properly deal with
-0 up until now, leading to suboptimal typing, i.e. for a simple case
like
Math.max(Math.round(x), 1)
TurboFan was unable to figure out that the result is definitely going
to be a positive integer in the range [1,inf] or NaN (assuming that
NumberOrOddball feedback is used for the value x).
Bug: v8:8015
Change-Id: I06e14a9c9b0b813eb214ace7749fcc6ab36bb66a
Reviewed-on: https://chromium-review.googlesource.com/1199304
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55570}
The new node is introduced for literal string addition and calling
String.prototype.concat in the typed lowering phase. It later might get optimized
away during redundancy elimination, keeping the performance of already existing
benchmarks with string addition. In case the operation is about to throw
(due to too long string being constructed) we just deoptimize, reusing
the interpreter logic for creating the error.
Modify relevant mjsunit and unit tests for string concatenation.
Bug: v8:7902
Change-Id: Ie97d39534df4480fa8d4fe3ba276d02ed5e750e3
Reviewed-on: https://chromium-review.googlesource.com/1193342
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55482}
