Skip to content

V
V8
Switch branch/tag
  1. 08 Oct, 2018 14 commits
    • Benedikt Meurer's avatar
      [turbofan] Escape analysis support for LoadElement with variable index. · 3e43ded9
      Benedikt Meurer authored 
      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: 's avatarTobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56442}
      3e43ded9
    • Benedikt Meurer's avatar
      [turbofan] Introduce the notion of context-sensitivity for JS operators. · 51274d3c
      Benedikt Meurer authored 
      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/1267176Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56441}
      51274d3c
    • Clemens Hammacher's avatar
      [wasm] Fix code space management · eeb15e84
      Clemens Hammacher authored 
      On windows, the {NativeModule::committed_code_space_} counter can underflow because
of a bug. This propagates to {WasmCodeManager::remaining_uncommitted_code_space_},
which can lead to over-allocation (more than {kMaxWasmCodeMemory} bytes of code
space per module).

We were also seeing this bug on UMA data (>1024 MB code space usage).

R=ahaas@chromium.org

Bug: chromium:893096

Change-Id: If3c9b3e7bdc9fc3caf1eccae991123409718b90f
Reviewed-on: https://chromium-review.googlesource.com/c/1267943Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56440}
      eeb15e84
    • Toon Verwaest's avatar
      [parser] Cleanup PatternRewriter construction · 9a4c29f9
      Toon Verwaest authored 
      This also precomputes "declares parameter containing sloppy eval" and reorders
fields for better packing.

Change-Id: I598ed658f79e7d83f6b844236fc60518d9cf9f26
Reviewed-on: https://chromium-review.googlesource.com/c/1267940Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56439}
      9a4c29f9
    • Hai Dang's avatar
      Reland "Add fast path for spreading primitive strings." · 760eed05
      Hai Dang authored 
      This is a reland of ef2a19a2.
Use AllocateJSArray to avoid allocating an empty fixed array.

Original change's description:
> Add fast path for spreading primitive strings.
>
> This improves the performance on primitive strings of
> IterableToListWithSymbolLookup, which implements the
> CreateArrayFromIterable bytecode. The fast path is only
> taken if the string iterator protector is valid (that is,
> String.prototype[Symbol.iterator] and
> String.prototype[Symbol.iterator]().next are untouched).
>
> This brings spreading of primitive strings closer to the
> performance of the string iterator optimizations.
> (see https://docs.google.com/document/d/13z1fvRVpe_oEroplXEEX0a3WK94fhXorHjcOMsDmR-8/).
>
> Bug: chromium:881273, v8:7980
> Change-Id: Ic8d8619da2f2afcc9346203613a844f62653fd7a
> Reviewed-on: https://chromium-review.googlesource.com/1243110
> Commit-Queue: Hai Dang <dhai@google.com>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56329}

Bug: chromium:881273, v8:7980
Change-Id: I746c57ddfc300e1032057b5125bc824adf5c2cd3
Reviewed-on: https://chromium-review.googlesource.com/c/1267497
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56438}
      760eed05
    • Mathias Bynens's avatar
      Roll Test262 · c74db9f6
      Mathias Bynens authored 
      Bug: v8:7834
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ie588d032136b164a2e1bcfacf3c22b1a3428f20e
Reviewed-on: https://chromium-review.googlesource.com/c/1262676
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: 's avatarSathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56437}
      c74db9f6
    • Jaroslav Sevcik's avatar
      [turbofan] Make map check success label non-deferred. · 6ca0bcca
      Jaroslav Sevcik authored 
      Bug: chromium:893058
Change-Id: I679c5e645eda5e8e5eb97fa873d0e2ee8ce61e11
Reviewed-on: https://chromium-review.googlesource.com/c/1267938
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56436}
      6ca0bcca
    • Michael Starzinger's avatar
      [wasm] Fix disassembly of wasm-stub call while on-heap. · 15d35405
      Michael Starzinger authored 
      R=clemensh@chromium.org
BUG=v8:8263

Change-Id: I6149cc6b353d4676a4b9170c906fe37822020217
Reviewed-on: https://chromium-review.googlesource.com/c/1267941Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56435}
      15d35405
    • Georg Neis's avatar
      [turbofan] Don't constant-fold typed-array with on-heap buffer. · 6c86b81a
      Georg Neis authored 
      When generating code for element accesses, we used to constant-fold
JSTypedArray receivers even when their buffers were on the JS heap.
This required a call to MaterializeArrayBuffer, which hinders
background compilation. Since the benefit of this optimization is
believed to be small, we decided to remove it.

Bug: v8:7790
Change-Id: I28d3a57b3d8f5b58b6e00e0bb8328b682a6fbd88
Reviewed-on: https://chromium-review.googlesource.com/c/1256831
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56434}
      6c86b81a
    • Georg Neis's avatar
      Make JSTypedArray::length() and length_value() ignore neutering. · 3b64764b
      Georg Neis authored 
      Return the actual length even when the buffer is neutered (we used
to return 0). This avoids confusion and makes the behavior consistent
with byte_offset() and byte_length().

Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I998f12fa4a428f8555f62e1535247f571ab053f2
Reviewed-on: https://chromium-review.googlesource.com/c/1256768Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56433}
      3b64764b
    • Maya Lekova's avatar
      [async-await] Fix global-buffer-overflow issue when loading flag · 890fd9c8
      Maya Lekova authored 
      Bug: chromium:892858
Change-Id: I97b0b239e3ee0a9073fdbd609fb26271dda64d6d
Reviewed-on: https://chromium-review.googlesource.com/c/1267936
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56432}
      890fd9c8
    • Jaroslav Sevcik's avatar
      [profview] Use function/script id rather than the name string. · 2218d418
      Jaroslav Sevcik authored 
      Using function ids is more reliable since there can be several functions
or scripts with the same name. Also, that way we do not have to parse
anything.

Change-Id: If657141d0d6e27dabb49456e0275cce65e753541
Reviewed-on: https://chromium-review.googlesource.com/c/1267496Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56431}
      2218d418
    • Benedikt Meurer's avatar
      [turbofan] Don't introduce unnecessary x===true comparisons. · 6c505fb9
      Benedikt Meurer authored 
      In the JSCallReducer, the lowering for Array#filter(), Array#some() and
Array#every() properly converted the outcome of the predicate call to
boolean using the ToBoolean conversion, but then also added a redundant
ReferenceEqual comparison with true. This particular pattern is not
optimized by TurboFan, since it can never happen using the regular
comparison machinery. So remove the unnecessary ReferenceEqual and just
do the ToBoolean in the JSCallReducer.

Bug: v8:8238
Change-Id: Ic2585431b4b75d3d5f978c85156cfb19738b7ae6
Reviewed-on: https://chromium-review.googlesource.com/c/1267177Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56430}
      6c505fb9
    • v8-ci-autoroll-builder's avatar
      Update V8 DEPS. · 9718084d
      v8-ci-autoroll-builder authored 
      Rolling v8/build: https://chromium.googlesource.com/chromium/src/build/+log/86e6b5e..63f397a

TBR=machenbach@chromium.org,hablich@chromium.org,sergiyb@chromium.org

Change-Id: Iff11ed400b0e9440fa03f8b783e4ae4308c0166c
Reviewed-on: https://chromium-review.googlesource.com/c/1267656
Commit-Queue: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Reviewed-by: 's avatarv8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#56429}
      9718084d
  3. 07 Oct, 2018 3 commits
    • Benedikt Meurer's avatar
      [turbofan] Eliminate redundant Smi checks around array accesses. · bcdede0c
      Benedikt Meurer authored 
      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/1246181Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56428}
      bcdede0c
    • Benedikt Meurer's avatar
      Revert "[turbofan] Do not consume SignedSmall feedback in TurboFan anymore." · 248fd5ff
      Benedikt Meurer authored 
      This reverts commit 4fd92b25.

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/1267175Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56427}
      248fd5ff
    • v8-ci-autoroll-builder's avatar
      Update V8 DEPS. · c73c2dea
      v8-ci-autoroll-builder authored 
      Rolling v8/build: https://chromium.googlesource.com/chromium/src/build/+log/a092193..86e6b5e

TBR=machenbach@chromium.org,hablich@chromium.org,sergiyb@chromium.org

Change-Id: I70e960aca4160188ecc4100d286110f91f013964
Reviewed-on: https://chromium-review.googlesource.com/c/1266854
Commit-Queue: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Reviewed-by: 's avatarv8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#56426}
      c73c2dea
  5. 06 Oct, 2018 5 commits
  7. 05 Oct, 2018 18 commits