1. 21 Mar, 2022 1 commit
  2. 22 Dec, 2021 1 commit
    • Shu-yu Guo's avatar
      [compiler] Fix typing JSLoadNamed of private brands · d19a707d
      Shu-yu Guo authored
      Private method loads are compiled to a named load of a private brand,
      which always loads a BlockContext. This BlockContext holds the private
      methods common to all instances of a class. TurboFan currently considers
      JSLoadNamed to be of Type::NonInternal(). Private methods break this
      assumption, since BlockContext is of Type::OtherInternal().
      
      This CL changes the typing of JSLoadNamed of private brands to be
      Type::OtherInternal().
      
      Bug: v8:12500
      Change-Id: I91f39747bf9422bd419d299f44152f567d8be8db
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3351167Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
      Commit-Queue: Shu-yu Guo <syg@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78431}
      d19a707d
  3. 13 Oct, 2021 1 commit
    • Joyee Cheung's avatar
      [class] Add IC support for defining class fields to replace runtime call · 713ebae3
      Joyee Cheung authored
      Introduces several new runtime mechanics for defining private fields,
      including:
        - Bytecode StaKeyedPropertyAsDefine
        - Builtins StoreOwnIC{Trampoline|Baseline|_NoFeedback}
        - Builtins KeyedDefineOwnIC{Trampoline|Baseline|_Megamorphic}
        - TurboFan IR opcode JSDefineProperty
      
      These new operations can reduce a runtime call per class field into a
      more traditional Store equivalent. In the microbenchmarks, this
      results in a substantial win over the status quo (~8x benchmark score
      for single fields with the changes, ~20x with multiple fields).
      
      The TurboFan JSDefineProperty op is lowered in
      JSNativeContextSpecialization, however this required some hacks.
      Because private fields are defined as DONT_ENUM when added to the
      object, we can't find a suitable transition using the typical data
      property (NONE) flags. I've added a mechanism to specify the required
      PropertyAttributes for the transition we want to look up.
      
      Details:
      
      New bytecodes:
        - StaKeyedPropertyAsDefine, which is essentially StaKeyedProperty
          but with a different IC builtin (KeyedDefineOwnIC). This is a
          bytecode rather than a flag for the existing StaKeyedProperty in
          order to avoid impacting typical keyed stores in any way due to
          additional branching and testing.
      
      New builtins:
        - StoreOwnIC{TTrampoline|Baseline|_NoFeedback} is now used for
          StaNamedOwnProperty. Unlike the regular StoreIC, this variant will
          no longer look up the property name in the prototype.
          In adddition, this CL changes an assumption that
          StoreNamedOwnProperty can't result in a map transition, as we
          can't rely on the property already being present in the Map due
          to an object literal boilerplate.
      
          In the context of class features, this replaces the runtime
          function %CreateDataProperty().
      
        - KeyedDefineOwnIC{Trampoline|Baseline|_Megamorphic} is used by the
          new StaKeyedPropertyAsDefine bytecode. This is similar to an
          ordinary KeyedStoreIC, but will not check the prototype for
          setters, and for private fields, will take the slow path if the
          field already exists.
      
          In the context of class features, this replaces the runtime
          function %AddPrivateField().
      
      TurboFan IR:
        - JSDefineProperty is introduced to represent a situation where we
          need to use "Define" semantics, in particular, it codifies that we
          do not consult the prototype chain, and the semantics relating to
          private fields are implied as well.
      
      R=leszeks@chromium.org, syg@chromium.org, rmcilroy@chromium.org
      
      Bug: v8:9888
      Change-Id: Idcc947585c0e612f9e8533aa4e2e0f8f0df8875d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2795831Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
      Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Reviewed-by: 's avatarShu-yu Guo <syg@chromium.org>
      Commit-Queue: Joyee Cheung <joyee@igalia.com>
      Cr-Commit-Position: refs/heads/main@{#77377}
      713ebae3
  4. 22 Jul, 2021 1 commit
  5. 20 Jul, 2021 2 commits
  6. 26 Apr, 2021 1 commit
  7. 22 Apr, 2021 1 commit
    • Jakob Gruber's avatar
      [compiler] Support GetPropertyAccessInfo in a concurrent setting · 1277bb5c
      Jakob Gruber authored
      Until this CL, the JSHeapBroker::GetPropertyAccessInfo (GPAI) process
      was as follows:
      
       1. GPAI is called on the main thread (MT) during the serialization
          phase to create and cache PAIs.
       2. GPAI is called again from the background thread (BT); only cached
          PAIs from step 1 are usable.
      
      As part of concurrent inlining, the goal is to move GPAI fully to the
      background thread. This CL takes a major step in that direction by
      making GPAI itself callable from the BT without resorting solely to PAIs
      that were previously cached on the MT.
      
      There are two main reasons why GPAI previously had to run on the MT:
      
       a) Concurrent access to Maps and other heap objects.
       b) Serialization and creation of ObjectRefs for objects discovered
          during GPAI.
      
      This CL addresses only reason a) and leaves b) for future work. This
      is done by keeping the two-pass approach, s.t. the initial call of
      GPAI on the MT discovers and serializes objects. We then clear all
      cached PAIs. The second call of GPAI on the BT thus runs full logic in a
      concurrent setting.
      
      Once all relevant objects (= maps and prototypes) no longer require
      MT-serialization, reason b) is also addressed and the first pass can be
      removed.
      
      The new logic is implemented behind the runtime flag
      --turbo-concurrent-get-property-access-info (default true), intended
      to be removed in the future.
      
      Bug: v8:7790
      Change-Id: Idbdbfe091d7316529246a686bb6d71c2a0f06f8b
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2817793
      Commit-Queue: Jakob Gruber <jgruber@chromium.org>
      Auto-Submit: Jakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#74120}
      1277bb5c
  8. 01 Apr, 2021 1 commit
    • Jakob Gruber's avatar
      [compiler] Add the MapUpdater lock · 605f9875
      Jakob Gruber authored
      It's locked exclusively in the MapUpdater API methods, and locked
      shared in ComputePropertyAccessInfo (CPAI).
      
      This lock is a step towards running CPAI on background threads. The
      simple lock portion is landed separately in this CL to get an early
      signal on potential lock overhead perf impact.
      
      The lock is implemented and used very conservatively at the moment:
      
      - it's a single global lock (and not e.g. per-map).
      - it's locked for the entire method call duration (instead of only in
        relevant parts).
      
      Both points can potentially be improved in the future.
      
      Bug: v8:7790
      Change-Id: I073423497e01b4901101973387a19962f953a576
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2797286Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Commit-Queue: Jakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#73773}
      605f9875
  9. 23 Mar, 2021 4 commits
  10. 03 Mar, 2021 1 commit
    • Frank Emrich's avatar
      [dict-proto] TF support for constants in dictionary mode protos, pt. 1 · a3ad3529
      Frank Emrich authored
      This CL is the first in a series that implements Turbofan support for
      property accesses satisfying the following conditions:
      1. The holder is a dictionary mode object.
      2. The holder is a prototype.
      3. The access is a load.
      
      This feature will only be enabled if the build flag
      v8_dict_property_const_tracking is set.
      
      This particular CL does the following:
      
      a) In PropertyAccessInfo::Kind, rename kDataConstant and
      kAccessorConstant to kFastDataConstant and kFastAccessorConstant,
      respectively, to indicate that these kinds are used for fast mode
      holders.
      
      b) In PropertyAccessInfo::Kind, add kDictionaryProtoDataConstant and
      kDictionaryProtoAccessorConstant, which will be used for dictionary
      mode holders (which must also be prototypes, as stated  above).
      
      c) Add a member dictionary_index_ to PropertyAccessInfo, which is
      used by the kinds mentioned in b)
      
      Bug: v8:11248
      Change-Id: Id1c10215aab287066a9765756f112c8035141013
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2718228
      Commit-Queue: Frank Emrich <emrich@google.com>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#73169}
      a3ad3529
  11. 10 Nov, 2020 1 commit
  12. 09 Nov, 2020 2 commits
  13. 06 Nov, 2020 1 commit
  14. 05 Nov, 2020 2 commits
  15. 28 Jul, 2020 1 commit
  16. 11 Oct, 2019 1 commit
  17. 19 Aug, 2019 3 commits
    • Georg Neis's avatar
      Reland "[turbofan] Various serializer/broker improvements" · 4b1af9fc
      Georg Neis authored
      This is a reland of 29585a06 after
      removing an incorrect DCHECK.
      
      Original change's description:
      > [turbofan] Various serializer/broker improvements
      >
      > They are all somewhat entangled, sorry for the big CL.
      >
      > - Brokerize remaining feedback vector slots.
      > - Introduce Hints::SingleConstant helper.
      > - Introduce SerializationPolicy enum.
      > - Eliminate use of nullptr for megamorphic load/store ic feedback.
      >   Instead use the corresponding ProcessedFeedback with an empty list
      >   of maps or the like. new class MegamorphicFeedback.
      > - Separate processing of feedback from serialization. This eliminates
      >   code duplication.
      > - Be very careful when clearing hints not to overwrite hints that are
      >   being processed.
      > - Move AccessInfos out of NamedAccessFeedback. Always store them in
      >   property_access_infos_ map on broker. (This was actually unused
      >   before, somewhat by mistake.)
      > - Support map inference in concurrent inlining. Rewrite
      >   ElementAccessFeedback such that we can refine it with the set of
      >   inferred maps.
      >
      > TBR: mvstanton@chromium.org
      > Change-Id: I05e9eb250bdffc6dff29db01742550a86a41cb31
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1752853
      > Commit-Queue: Georg Neis <neis@chromium.org>
      > Reviewed-by: Georg Neis <neis@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#63232}
      
      TBR: mvstanton@chromium.org
      Bug: v8:7790
      Change-Id: Ia4acd31b339a941ee065e1ae4835bb7b85d5685e
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1758319Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Commit-Queue: Georg Neis <neis@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63241}
      4b1af9fc
    • Maya Lekova's avatar
      Revert "[turbofan] Various serializer/broker improvements" · 0645b26a
      Maya Lekova authored
      This reverts commit 29585a06.
      
      Reason for revert: Breaks GC stress bots - 
      https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/24009
      https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/27281
      
      Original change's description:
      > [turbofan] Various serializer/broker improvements
      > 
      > They are all somewhat entangled, sorry for the big CL.
      > 
      > - Brokerize remaining feedback vector slots.
      > - Introduce Hints::SingleConstant helper.
      > - Introduce SerializationPolicy enum.
      > - Eliminate use of nullptr for megamorphic load/store ic feedback.
      >   Instead use the corresponding ProcessedFeedback with an empty list
      >   of maps or the like. new class MegamorphicFeedback.
      > - Separate processing of feedback from serialization. This eliminates
      >   code duplication.
      > - Be very careful when clearing hints not to overwrite hints that are
      >   being processed.
      > - Move AccessInfos out of NamedAccessFeedback. Always store them in
      >   property_access_infos_ map on broker. (This was actually unused
      >   before, somewhat by mistake.)
      > - Support map inference in concurrent inlining. Rewrite
      >   ElementAccessFeedback such that we can refine it with the set of
      >   inferred maps.
      > 
      > TBR: mvstanton@chromium.org
      > Change-Id: I05e9eb250bdffc6dff29db01742550a86a41cb31
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1752853
      > Commit-Queue: Georg Neis <neis@chromium.org>
      > Reviewed-by: Georg Neis <neis@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#63232}
      
      TBR=mvstanton@chromium.org,neis@chromium.org
      
      Change-Id: I88625d92fddf993db63661666c59af05a47b2b58
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1758314Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
      Commit-Queue: Maya Lekova <mslekova@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63237}
      0645b26a
    • Georg Neis's avatar
      [turbofan] Various serializer/broker improvements · 29585a06
      Georg Neis authored
      They are all somewhat entangled, sorry for the big CL.
      
      - Brokerize remaining feedback vector slots.
      - Introduce Hints::SingleConstant helper.
      - Introduce SerializationPolicy enum.
      - Eliminate use of nullptr for megamorphic load/store ic feedback.
        Instead use the corresponding ProcessedFeedback with an empty list
        of maps or the like. new class MegamorphicFeedback.
      - Separate processing of feedback from serialization. This eliminates
        code duplication.
      - Be very careful when clearing hints not to overwrite hints that are
        being processed.
      - Move AccessInfos out of NamedAccessFeedback. Always store them in
        property_access_infos_ map on broker. (This was actually unused
        before, somewhat by mistake.)
      - Support map inference in concurrent inlining. Rewrite
        ElementAccessFeedback such that we can refine it with the set of
        inferred maps.
      
      TBR: mvstanton@chromium.org
      Change-Id: I05e9eb250bdffc6dff29db01742550a86a41cb31
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1752853
      Commit-Queue: Georg Neis <neis@chromium.org>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63232}
      29585a06
  18. 16 Aug, 2019 1 commit
    • Georg Schmid's avatar
      [turbofan] Track field owner maps during load elimination · f85826ea
      Georg Schmid authored
      This CL adds additional information in PropertyAccessInfos and FieldAccesses about the map that introduced the accessed field. We use this information to prevent load elimination from incorrectly optimizing certain accesses marked const.
      
      Prior to this CL, load elimination simply stored information about eliminatable field accesses based on objects (identified by nodes in the graph) and offsets (i.e., statically known ones). In the presence of const stores and loads this is insufficient, since a single object (in the above sense) may contain distinct *const* properties at the same offset throughout its lifetime. As an example, consider the following piece of code:
      
          let obj = {};
          obj.a = 0;
          obj[1024] = 1;  // An offset of >=1024 forces an elements-kind transition
          delete obj.a;
          obj.b = 2;
          assertEquals(obj.b, 2);
      
      In this scenario, *both* the first ('obj.a = 0') and the second ('obj.b = 2') store to a field will be marked const by the runtime. The reason that storing to 'a' above ends up being marked const, is that 'a' before and after the elements-kind transition is encoded in separate transition trees. Removing 'a' ('delete obj.a') only invalidates const-ness in the dictionary-elements transition tree; not the holey-elements one used at the time of 'obj.a = 0'.
      
      The above situation on its own violates an invariant in load elimination. Namely, we assume that for the same object and offset, we will never encounter two const stores. One can extend the above snippet to coax load-elimination into producing incorrect results. For instance, by "hiding" 'obj.b = 2' in an unoptimized function call, the consecutive load from 'b' will incorrectly produce 0, violating the assert.
      
      R=neis@chromium.org, tebbi@chromium.org
      
      Bug: chromium:980183, chromium:983764
      Change-Id: I576a9c7efd416fa9db6daff1f42d483e4bd369b4
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751346
      Commit-Queue: Georg Schmid <gsps@google.com>
      Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63226}
      f85826ea
  19. 17 Jul, 2019 1 commit
  20. 05 Jul, 2019 1 commit
  21. 21 Jun, 2019 1 commit
  22. 27 May, 2019 1 commit
    • Georg Schmid's avatar
      Reland "Make LoadElimination aware of const fields (Part 2; stores)" · 85f257f4
      Georg Schmid authored
      This is a reland of e588ff10
      
      The only change over the original CL is found in JSCreateLowering::AllocateFastLiteral. We now guard against boilerplate values for unboxed double fields that *look* like legitimate initial values, but should really be kHoleNanInt64 instead.
      
      The underlying problem certainly existed before, but an invariant added to LoadElimination in this CL caused a Chromium layout test to fail. The change in this reland is therefore a workaround, the root cause remains to be fixed. Specifically, we find that a pointer to the undefined value oddball is sometimes reinterpreted as a double and assigned as a boilerplate value. @jarin suspects that this stems from in-place map updates.
      
      Original change's description:
      > Make LoadElimination aware of const fields (Part 2; stores)
      >
      > Adds const information to store field accesses and uses it in load elimination
      >
      > Change-Id: I00765c854c95c955dabd78557463267b95f75eef
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1611543
      > Reviewed-by: Georg Neis <neis@chromium.org>
      > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
      > Commit-Queue: Georg Schmid <gsps@google.com>
      > Cr-Commit-Position: refs/heads/master@{#61796}
      
      Change-Id: Ie388754890024a3ca7d10c9d4d7391442655b426
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1630676Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
      Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Commit-Queue: Georg Schmid <gsps@google.com>
      Cr-Commit-Position: refs/heads/master@{#61838}
      85f257f4
  23. 24 May, 2019 1 commit
  24. 23 May, 2019 2 commits
  25. 22 May, 2019 1 commit
  26. 21 May, 2019 1 commit
  27. 20 May, 2019 1 commit
  28. 16 May, 2019 1 commit
  29. 15 May, 2019 1 commit
  30. 10 May, 2019 1 commit
  31. 27 Apr, 2019 1 commit