1. 14 Jul, 2020 1 commit
  2. 10 Jul, 2020 1 commit
  3. 09 Jul, 2020 1 commit
  4. 04 Jun, 2020 2 commits
    • Nico Hartmann's avatar
      [turbofan] Fix lost exception on BigInt ops · ca54b833
      Nico Hartmann authored
      Speculative BigInt addition fails to throw the expected exception
      when called with non-BigInt inputs when the result of the computation
      is unused. In paricular, this CL does:
       - Remove kNoThrow on speculative BigInt operators
       - Fix AddWithFeedback to not lose type feedback if builtin throws
         to elide existing deopt loops
       - Add handling of TypeCheckKind in RepresentationChanger where this
         was previously ignored
      
      Bug: chromium:1073440
      Change-Id: I953a5b790fc3b37a6824f0b6546a0488c51fbb3b
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2228493Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
      Reviewed-by: 's avatarMythri Alle <mythria@chromium.org>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Commit-Queue: Mythri Alle <mythria@chromium.org>
      Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68181}
      ca54b833
    • Mythri A's avatar
      [ic] Fix a bug in StoreOwnIC when storing NaN values · b6133551
      Mythri A authored
      We use StoreOwnIC to initialize the object after creating a new object
      with CreateObjectLiteral. CreateObjectLiteral stores kHoleNaNInt64
      to indicate an uninitialized double field. When we actually try
      to store a NaN value into that field later using StoreOwnIC, IC avoids
      actually storing the new value since the existing value is "same as"
      the value we try to write. The float comparison treats all NaNs as
      equal. In this particular case, we should actually store the new value
      since kHoleNaNInt64 value is used to represent an uninitialized field.
      
      This cl just stores the new value even when the existing value is same
      as the new value for double fields. The check is still required to
      correctly track const fields.
      
      Bug: chromium:1082293
      Change-Id: Ib37061802f2403545cffa6d6fef08be074b0825d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2228886Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Commit-Queue: Mythri Alle <mythria@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68167}
      b6133551
  5. 28 May, 2020 1 commit
  6. 27 May, 2020 1 commit
    • Jakob Gruber's avatar
      [ic] Refactor unary op assembler implementations · e1427aea
      Jakob Gruber authored
      This CL brings unary op assembler structure closer to that of binary
      ops assemblers:
      
      - Decrement, Increment, Negate call into UnaryOpWithFeedback,
      - which takes lambdas specifying smi, float, and bigint logic.
      - BitwiseNot is different in that it still dispatches using
        TaggedToWOrd32OrBigIntWithFeedback.
      - These methods are all implemented in the (hidden)
        UnaryOpAssemblerImpl class.
      - The header only exposes UnaryOpAssembler with the bare minimum of
        API.
      
      The last point is the remaining major divergence from binary op
      assemblers. I just like how this avoids useless implementation details
      in the header.
      
      Bug: v8:8888
      Change-Id: I0ac4695483950356885301234d58c1900904aa92
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2214830
      Commit-Queue: Jakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68004}
      e1427aea
  7. 25 May, 2020 1 commit
  8. 18 May, 2020 1 commit
  9. 13 May, 2020 1 commit
  10. 11 May, 2020 1 commit
  11. 30 Apr, 2020 1 commit
  12. 20 Apr, 2020 1 commit
  13. 15 Apr, 2020 2 commits
    • Leszek Swirski's avatar
      [turbofan] Avoid megamorphic loads for zero-map mono/polymorphic sites · 8428feed
      Leszek Swirski authored
      Soft-deopt for mono/polymorphic property accesses that don't have any
      maps, and only allow zero-map feedback to be monomorphic. This makes
      sure we only emit a megamorphic LoadIC builtin call if the IC was
      actually megamorphic.
      
      JSGenericLowering assumed that zero maps meant that a load site is
      megamorphic. However, it can be the case that the call-site is
      monomorphic or polymorphic, and the maps had died. In this case we don't
      want to call the megamorphic IC builtin, as on a stub cache miss we
      fallback to a normal LoadIC miss, which can record mono/polymorphic
      feedback in the IC. After this, we'll enter a miss loop in the
      megamorphic load builtin, and worse the LoadIC assumes that there's
      something "wrong" with the feedback, so it'll keep trying to reconfigure
      the handler (possibly allocating new load handlers if this is a
      prototype field access).
      
      As a drive-by, rewrite GetRelevantReceiverMaps to be an in-place
      filtering of the maps rather than copying them.
      
      Change-Id: I0c25bfa606367fa81c43223bbd56cdadb5e789ef
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2150586Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Commit-Queue: Georg Neis <neis@chromium.org>
      Auto-Submit: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67152}
      8428feed
    • Mythri A's avatar
      [ic] Use slow stub when storing non-existent properties to global object · d11292fc
      Mythri A authored
      In strict mode stores to non-existent properties throw. We should not
      install a handler with the property cell for such stores. These handlers
      would expect that the value exists when they see a property cell. If
      this property cell gets invalidated later, it appears as if it is a
      valid property cell with undefined value. This leads to an incorrect
      behaviour. This cl checks if we are in strict mode and uses a slow
      stub in such cases.
      
      Bug: chromium:1067757
      Change-Id: I543c6a6931530bfb13cc9a33d1dabaa756489fd1
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2142255
      Commit-Queue: Mythri Alle <mythria@chromium.org>
      Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67151}
      d11292fc
  14. 03 Apr, 2020 1 commit
    • Bruce Dawson's avatar
      Revert "Speculative fix to crashes from a CPU bug" · e84e9b92
      Bruce Dawson authored
      This reverts commit 10360127.
      
      Reason for revert: This fix only had moderate impact and the
      underlying CPU bug has now been addressed.
      
      Original change's description:
      > Speculative fix to crashes from a CPU bug
      > 
      > For the last few months Chrome has been seeing many "impossible" crashes
      > on Intel Gemini Lake, family 6 model 122 stepping 1 CPUs. These crashes
      > only happen with 64-bit Chrome and only happen in the prologue of two
      > functions. The crashes come and go across different Chrome versions.
      > Analysis of most of the crashes shows that the address of the crashing
      > instruction follows some patterns:
      > 
      > When crashing in GetFieldIndex() the last byte of the address is always
      > 1c, 5c, 9c, or dc.
      > 
      > When crashing in UpdateCaches (fewer unique samples) the last byte of
      > the address is always 5d or 9d.
      > 
      > The address of the function is 0xc or 0xd bytes earlier so the crashing
      > functions always start with an address that ends in 10, 50, 90, or d0.
      > 
      > Those addresses are for the crashes on a load of the __security_cookie.
      > The crashes also occasionally happen on the two instructions that follow
      > the __security_cookie load in which case the crashing instruction's
      > address has been seen to end with 23 or a3. This corresponds to a
      > function start address of 10 or 90.
      > 
      > Since the crash involves reading incorrect instruction bytes when
      > crossing a 16-byte boundary and since the crash appears to only happen
      > with particular 16-byte alignments it seems reasonable to force the
      > function's alignments to a multiple of 32 to see if this reliably
      > avoids the crashes. This change uses the gcc/clang __attribute__
      > directive to force 32-byte alignment. I have tested this change enough to
      > verify that it triggers the desired alignment (with up to 31 "int 3"
      > instructions added for padding) but since I have never reproduced this
      > crash I have no way of testing its efficacy.
      > 
      > Bug: chromium:968683, chromium:964273
      > Change-Id: Ia6e1c6d1e044b84d274817374b25523303e78b51
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803775
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#63804}
      
      TBR=brucedawson@chromium.org,verwaest@chromium.org
      
      # Not skipping CQ checks because original CL landed > 1 day ago.
      
      Bug: chromium:968683, chromium:964273
      Change-Id: I150ecfebeff95e8f63dbba74d78491867dc17736
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2134728
      Commit-Queue: Toon Verwaest <verwaest@chromium.org>
      Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#66977}
      e84e9b92
  15. 02 Apr, 2020 1 commit
    • Mythri A's avatar
      [ic] Use the existing prototype validity cell when recomputing handlers · 800c294c
      Mythri A authored
      For keyed stores we recompute handlers based on the receiver maps
      we have seen. This is done so that we can transition to the most generic
      elements kind we have seen so far. When we recompute this handlers we
      get a new prototype validity cell and ignore the existing cell. This
      leads to incorrect behaviour if the cell was invalid. Recomputing the
      handler may be extra work which is not worth doing at this point. So
      we just reuse the existing validity cell and let the IC recompute the
      handler if we see the map again.
      
      Bug: chromium:1053939
      Change-Id: Ifc891d70f5a4b8b774238e12fb40e29b4d174e37
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2122032
      Commit-Queue: Mythri Alle <mythria@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#66963}
      800c294c
  16. 31 Mar, 2020 1 commit
  17. 20 Mar, 2020 1 commit
  18. 13 Mar, 2020 1 commit
  19. 09 Mar, 2020 1 commit
  20. 04 Mar, 2020 1 commit
  21. 20 Feb, 2020 1 commit
  22. 19 Feb, 2020 2 commits
  23. 17 Feb, 2020 1 commit
  24. 14 Feb, 2020 1 commit
  25. 13 Feb, 2020 1 commit
  26. 14 Jan, 2020 1 commit
  27. 13 Jan, 2020 2 commits
  28. 10 Jan, 2020 2 commits
    • Santiago Aboy Solanes's avatar
      [cleanup][CSA] TNodify LoadPropertyFromFastObject and related functions · 68376676
      Santiago Aboy Solanes authored
      Related ones are TryGetOwnProperty and CallGetterIfAccessor.
      
      Bug: v8:10021
      Change-Id: I1b65c4260ab48b4431fa2b84a8be5789f24fa800
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993960
      Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
      Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#65704}
      68376676
    • Seth Brenith's avatar
      [torque] move more bitfield definitions to Torque · 87c16da5
      Seth Brenith authored
      This change moves the definitions of the bitfield flags used by Symbol
      and Map to Torque. Symbol could directly follow the pattern established
      by SharedFunctionInfo, but Map required some other changes:
      - Until now, Torque bitfield definitions have required unsigned types. I
        thought that this would be the least-surprising behavior, since we
        never sign-extend when decoding bitfield values. However, I believe
        that the amount of churn involved in making ElementsKind be unsigned
        outweighs the benefit we were getting from this restriction (and
        similar difficulties are likely to arise in converting other bitfield
        structs to Torque), so this CL updates Torque to allow signed bitfield
        values.
      - If we try to make Map extend from all of the generated classes that
        define its flags, we end up with class sizing problems because some
        compilers only apply empty base class optimization to the first in a
        row of empty base classes. We could work around this issue by
        generating macros instead of classes, but I took this as an
        opportunity for a minor clean-up instead: rather than having bitfield
        definitions for several different bitfield structs all jumbled
        together in Map, they can be split up. I think this makes the code a
        little easier to follow, but if others disagree I'm happy to implement
        macro generation instead.
      
      Change-Id: Ibf339b0be97f72d740bf1daa8300b471912faeba
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1988934Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
      Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      Cr-Commit-Position: refs/heads/master@{#65701}
      87c16da5
  29. 09 Jan, 2020 1 commit
  30. 07 Jan, 2020 1 commit
  31. 18 Dec, 2019 1 commit
    • Leszek Swirski's avatar
      Reland "[ic] Load name/context lazily in LdaNamedProperty" · b21cda74
      Leszek Swirski authored
      This reverts commit 5377e72c.
      
      Reason for revert: Looks like the relevant graphs didn't recover after
      this revert, which suggests that the regression was an unrelated
      secondary effect. Relanding the original change since the revert did
      cause some microbenchmark regressions.
      
      Original change's description:
      > Revert "[ic] Load name/context lazily in LdaNamedProperty"
      > 
      > This reverts commit 347092ac.
      > 
      > Not a clean revert, since other changes got baked on top, but rather
      > a manual removal of LoadLazyICParameters.
      > 
      > Reason for revert: Seems to actually regress bindings perf tests (see
      > bugs and https://chromeperf.appspot.com/group_report?rev=62539), doesn't
      > seem to improve performance elsewhere, and increases complexity.
      > 
      > Original change's description:
      > > [ic] Load name/context lazily in LdaNamedProperty
      > >
      > > Introduces LazyLoadICParameters which allow a LazyNode for context and
      > > name. These aren't used on the fast path, so we want to avoid reading
      > > them for both performance and register pressure reasons.
      > >
      > > Change-Id: Ifb637cf4782ce984feee9af503998e7539beb823
      > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1686665
      > > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      > > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > > Cr-Commit-Position: refs/heads/master@{#62539}
      > 
      > # Not skipping CQ checks because original CL landed > 1 day ago.
      > 
      > Bug: chromium:981797
      > Bug: chromium:982630
      > Change-Id: I88af764d17afb76d6e64b95a3d1e4aaa1c6c8978
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1934327
      > Auto-Submit: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Commit-Queue: Toon Verwaest <verwaest@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#65205}
      
      TBR=leszeks@chromium.org,verwaest@chromium.org
      
      # Not skipping CQ checks because original CL landed > 1 day ago.
      
      Bug: chromium:981797, chromium:982630, v8:10059
      Change-Id: I13754de06c83439e03e22cfaa7a14ce454076db9
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1973730Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#65499}
      b21cda74
  32. 27 Nov, 2019 1 commit
    • Leszek Swirski's avatar
      Revert "[ic] Load name/context lazily in LdaNamedProperty" · 5377e72c
      Leszek Swirski authored
      This reverts commit 347092ac.
      
      Not a clean revert, since other changes got baked on top, but rather
      a manual removal of LoadLazyICParameters.
      
      Reason for revert: Seems to actually regress bindings perf tests (see
      bugs and https://chromeperf.appspot.com/group_report?rev=62539), doesn't
      seem to improve performance elsewhere, and increases complexity.
      
      Original change's description:
      > [ic] Load name/context lazily in LdaNamedProperty
      >
      > Introduces LazyLoadICParameters which allow a LazyNode for context and
      > name. These aren't used on the fast path, so we want to avoid reading
      > them for both performance and register pressure reasons.
      >
      > Change-Id: Ifb637cf4782ce984feee9af503998e7539beb823
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1686665
      > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#62539}
      
      # Not skipping CQ checks because original CL landed > 1 day ago.
      
      Bug: chromium:981797
      Bug: chromium:982630
      Change-Id: I88af764d17afb76d6e64b95a3d1e4aaa1c6c8978
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1934327
      Auto-Submit: Leszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Commit-Queue: Toon Verwaest <verwaest@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#65205}
      5377e72c
  33. 26 Nov, 2019 2 commits
    • Suraj Sharma's avatar
      Modify the DCheck to avoid failures. · 2174ba9f
      Suraj Sharma authored
      Now since we also encode KeyedAccessStoreMode information in the slow
      handler for some cases, the DCheck can result in failure.The Check can
      result in failures for other cases of StoreSlow. Removing the DCHECK
      altogether, now verifying the correctness of the behavior using the
      Kind Bits of the Handler in the method GetKeyedAccessStoreMode.
      
      
      Bug: chromium:1027025, chromium:1028085
      Change-Id: I59acedbb499930e67ae5999d4bfd0f040a34b46e
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1929408Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Commit-Queue: Suraj Sharma <surshar@microsoft.com>
      Cr-Commit-Position: refs/heads/master@{#65185}
      2174ba9f
    • Jakob Kummerow's avatar
      More LookupIterator fixes after r65078 · 3ac7a3e5
      Jakob Kummerow authored
      (1) One more place in ic.cc must guard against "lookup->name()" calls
      when the LookupIterator might be in indexed mode.
      
      (2) Rather than burdening LookupIterator users with specifying
      "kGuaranteedNoTypedArray", we can do the corresponding calculation in
      the LookupIterator itself, which makes it robust towards any callers
      that haven't been updated (specifically, in Object.values).
      
      Bug: chromium:1027461,chromium:1028213
      Change-Id: I76b5d08e309fc2a694955b537adbeb5a30e681f7
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1936474Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#65177}
      3ac7a3e5
  34. 25 Nov, 2019 1 commit