1. 10 Jan, 2019 1 commit
  2. 13 Dec, 2018 1 commit
  3. 11 Dec, 2018 2 commits
  4. 06 Dec, 2018 1 commit
    • Jaroslav Sevcik's avatar
      [turbofan] Pin pure unreachable values to effect chain (in rep selection) · f27ac280
      Jaroslav Sevcik authored
      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/1365274Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58066}
      f27ac280
  5. 22 Nov, 2018 1 commit
    • Sigurd Schneider's avatar
      [turbofan] Make typed optimization more powerful · 9b0e4e13
      Sigurd Schneider authored
      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: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#57717}
      9b0e4e13
  6. 21 Nov, 2018 1 commit
  7. 19 Nov, 2018 2 commits
  8. 15 Nov, 2018 1 commit
  9. 08 Nov, 2018 1 commit
  10. 06 Nov, 2018 1 commit
  11. 30 Oct, 2018 2 commits
    • Hai Dang's avatar
      Fix typing of binary operators on BigInt. · c5c6b8bc
      Hai Dang authored
      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: 's avatarGeorg Neis <neis@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#57125}
      c5c6b8bc
    • Benedikt Meurer's avatar
      [turbofan] Enable loop peeling for various higher-order Array builtins. · d3f74c98
      Benedikt Meurer authored
      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/1305935Reviewed-by: 's avatarDaniel Clifford <danno@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#57110}
      d3f74c98
  12. 29 Oct, 2018 2 commits
    • Benedikt Meurer's avatar
      [turbofan] Add support for huge DataViews. · 15c31fe4
      Benedikt Meurer authored
      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/1303732Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#57095}
      15c31fe4
    • Benedikt Meurer's avatar
      [turbofan] Add Word64 support to NumberMin and NumberMax. · bb389dc7
      Benedikt Meurer authored
      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/1304535Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#57090}
      bb389dc7
  13. 24 Oct, 2018 2 commits
    • Benedikt Meurer's avatar
      [turbofan] ReceiverOrNullOrUndefined feedback for JSEqual. · f19c4a59
      Benedikt Meurer authored
      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/1297315Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56948}
      f19c4a59
    • Benedikt Meurer's avatar
      [turbofan] Collect and consume (ReceiverOr)Oddball feedback for StrictEqual. · 8f00d61d
      Benedikt Meurer authored
      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/1297309Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56925}
      8f00d61d
  14. 22 Oct, 2018 1 commit
  15. 18 Oct, 2018 1 commit
  16. 17 Oct, 2018 1 commit
  17. 15 Oct, 2018 1 commit
  18. 10 Oct, 2018 2 commits
    • Benedikt Meurer's avatar
      [turbofan] Improve NumberMultiply typing rule. · 585b4eef
      Benedikt Meurer authored
      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: 's avatarSigurd Schneider <sigurds@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56539}
      585b4eef
    • Benedikt Meurer's avatar
      [async] Improve async function handling. · 0038e5f0
      Benedikt Meurer authored
      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: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarSathya Gunasekaran <gsathya@chromium.org>
      Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
      Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56502}
      0038e5f0
  19. 08 Oct, 2018 2 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
  20. 07 Oct, 2018 2 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
  21. 05 Oct, 2018 2 commits
  22. 04 Oct, 2018 3 commits
    • Benedikt Meurer's avatar
      [turbofan] Propagate kIdentifyZeros correctly for modulus. · c4ada3de
      Benedikt Meurer authored
      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: 's avatarSigurd Schneider <sigurds@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56372}
      c4ada3de
    • Benedikt Meurer's avatar
      [turbofan] Unify number rounding operators. · 1d2a8e96
      Benedikt Meurer authored
      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/1260403Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56371}
      1d2a8e96
    • Benedikt Meurer's avatar
      [turbofan] Avoid unnecessary bit materialization in CheckedInt32Mod. · 7cd2cacf
      Benedikt Meurer authored
      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/1259063Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56370}
      7cd2cacf
  23. 01 Oct, 2018 3 commits
    • Benedikt Meurer's avatar
      [turbofan] Unify handling of zeros. · 8ead5698
      Benedikt Meurer authored
      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/1246172Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56325}
      8ead5698
    • Mathias Bynens's avatar
      Remove always-true --harmony-bigint runtime flag · f7d357b2
      Mathias Bynens authored
      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/1253604Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Commit-Queue: Mathias Bynens <mathias@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56310}
      f7d357b2
    • Jaroslav Sevcik's avatar
      [turbofan] Make sure we use only serialized elements kind transitions. · 56b6b6a8
      Jaroslav Sevcik authored
      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/1253105Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56307}
      56b6b6a8
  24. 28 Sep, 2018 1 commit
  25. 26 Sep, 2018 1 commit
  26. 21 Sep, 2018 2 commits
    • Benedikt Meurer's avatar
      [turbofan] Add missing test coverage for JSTypedLowering optimizations. · 9c0ef860
      Benedikt Meurer authored
      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/1238237Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56121}
      9c0ef860
    • Benedikt Meurer's avatar
      [turbofan] Refactor the CheckedInt32Div/CheckedUint32Div lowering. · ce7ec6ef
      Benedikt Meurer authored
      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/1236954Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56115}
      ce7ec6ef