• 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
Name
Last commit
Last update
benchmarks Loading commit data...
build_overrides Loading commit data...
custom_deps Loading commit data...
docs Loading commit data...
gni Loading commit data...
include Loading commit data...
infra Loading commit data...
samples Loading commit data...
src Loading commit data...
test Loading commit data...
testing Loading commit data...
third_party Loading commit data...
tools Loading commit data...
.clang-format Loading commit data...
.clang-tidy Loading commit data...
.editorconfig Loading commit data...
.git-blame-ignore-revs Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gn Loading commit data...
.vpython Loading commit data...
.ycm_extra_conf.py Loading commit data...
AUTHORS Loading commit data...
BUILD.gn Loading commit data...
CODE_OF_CONDUCT.md Loading commit data...
ChangeLog Loading commit data...
DEPS Loading commit data...
LICENSE Loading commit data...
LICENSE.fdlibm Loading commit data...
LICENSE.strongtalk Loading commit data...
LICENSE.v8 Loading commit data...
LICENSE.valgrind Loading commit data...
OWNERS Loading commit data...
PRESUBMIT.py Loading commit data...
README.md Loading commit data...
WATCHLISTS Loading commit data...
codereview.settings Loading commit data...
snapshot_toolchain.gni Loading commit data...