1. 22 Jul, 2021 1 commit
  2. 20 Jul, 2021 2 commits
  3. 26 Apr, 2021 1 commit
  4. 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
  5. 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
  6. 23 Mar, 2021 4 commits
  7. 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
  8. 10 Nov, 2020 1 commit
  9. 09 Nov, 2020 2 commits
  10. 06 Nov, 2020 1 commit
  11. 05 Nov, 2020 2 commits
  12. 28 Jul, 2020 1 commit
  13. 11 Oct, 2019 1 commit
  14. 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
  15. 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
  16. 17 Jul, 2019 1 commit
  17. 05 Jul, 2019 1 commit
  18. 21 Jun, 2019 1 commit
  19. 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
  20. 24 May, 2019 1 commit
  21. 23 May, 2019 2 commits
  22. 22 May, 2019 1 commit
  23. 21 May, 2019 1 commit
  24. 20 May, 2019 1 commit
  25. 16 May, 2019 1 commit
  26. 15 May, 2019 1 commit
  27. 10 May, 2019 1 commit
  28. 27 Apr, 2019 1 commit
  29. 26 Apr, 2019 1 commit
  30. 16 Apr, 2019 1 commit
  31. 14 Mar, 2019 1 commit
    • Georg Neis's avatar
      [turbofan] Preprocess feedback for global accesses (partially) · 04bb707e
      Georg Neis authored
      Main changes:
      - Rename ProcessedFeedback to ElementAccessFeedback and introduce a base class
        with the old name ProcessedFeedback.
      - Introduce another kind of ProcessedFeedback, namely GlobalAccessFeedback for
        the LoadGlobal/StoreGlobal IC. It's either a PropertyCell or a script context
        slot.
      - Produce such processed feedback in the serializer, when visiting LdaGlobal and
        similar bytecodes.
      - Consume it, and disallow heap access, in JSNativeContextSpecialization's
        ReduceJSLoadGlobal and ReduceJSStoreGlobal (for --concurrent-inlining).
      
      Minor changes:
      - Introduce a FeedbackSource class (pair of FeedbackVector and FeedbackSlot)
        that is used as the key of the processed feedback hash table. We already have
        two similar classes, FeedbackNexus and VectorSlotPair, but both are unsuitable
        for technical reasons (e.g. FeedbackNexus construction accesses the heap).
        Eventually we should remove VectorSlotPair.
      - Processed feedback is now returned as a pointer, which is nullptr if the
        original feedback wasn't interesting (e.g. megamorphic).
      
      The title says "partially" because the CL doesn't yet take into account named
      accesses where the receiver happens to be the global proxy.
      
      Bug: v8:7790
      Change-Id: I4404d98636b91a8f2d5667115944bae4773a4770
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1518184
      Commit-Queue: Georg Neis <neis@chromium.org>
      Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60240}
      04bb707e