Skip to content

V
V8
Switch branch/tag
  1. 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
  3. 10 Nov, 2020 1 commit
  5. 09 Nov, 2020 2 commits
  7. 06 Nov, 2020 1 commit
  9. 05 Nov, 2020 2 commits
  11. 28 Jul, 2020 1 commit
  13. 11 Oct, 2019 1 commit
  15. 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
  17. 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
  21. 05 Jul, 2019 1 commit
  23. 21 Jun, 2019 1 commit
  25. 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
  27. 24 May, 2019 1 commit
  29. 23 May, 2019 2 commits
  31. 22 May, 2019 1 commit
  33. 21 May, 2019 1 commit
  35. 20 May, 2019 1 commit
  37. 16 May, 2019 1 commit
  39. 15 May, 2019 1 commit
  41. 10 May, 2019 1 commit
  43. 27 Apr, 2019 1 commit
  45. 26 Apr, 2019 1 commit
  47. 16 Apr, 2019 1 commit
  49. 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
  51. 01 Mar, 2019 1 commit
    • Matt Gardner's avatar
      Reland "Optimize `in` operator" · 803ad324
      Matt Gardner authored 
      The original was reverted for breaking webkit layout tests:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064/30270

It also caused the following clusterfuzz failures:

chromium:935832
This was a correctness bug due to not properly handling the case of arrays with prototypes other
than Array.prototype. Accesses that were TheHole were not being handled property, both in bounds
holes in holey arrays and out of bounds on either holey or packed arrays. Handling was incorrect
both in access-assembler and in Turbofan.

chromium:935932
This bug was that there was no handling for Has checks on the global object. Turbofan was emitting
code for a store (the 'else' condition on 'access_mode == AccessMode::kLoad'). It hit a DCHECK in
debug builds but in release could show up in different places. This is the bug that caused the
webkit layout test failure that led to the revert.

Both bugs are fixed by in CL, and tests are added for those cases.

Bug: v8:8733, chromium:935932, chromium:935832
Change-Id: Iba0dfcfce6e15d2c0815a7670ece67bc13ba1925
Reviewed-on: https://chromium-review.googlesource.com/c/1493132Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Matt Gardner <magardn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#59958}
      803ad324
  53. 26 Feb, 2019 2 commits
  55. 25 Feb, 2019 1 commit
  57. 21 Feb, 2019 1 commit
  59. 13 Feb, 2019 1 commit
  61. 08 Feb, 2019 1 commit
  63. 02 Jan, 2019 1 commit
  65. 15 Oct, 2018 1 commit
  67. 07 Sep, 2018 1 commit
    • Benedikt Meurer's avatar
      [turbofan] Consistently use the StringLength operator. · 60d3f89d
      Benedikt Meurer authored 
      Previously all internal accesses to the String::length field in TurboFan
would use the StringLength operator, whereas explicit `string.length`
accesses from user JavaScript code would use LoadField operators instead.
This inconsistency led to redundant loads of the String::length, for
example in case of code like

```js
subject.substring(1, subject.length - 1)
```

where the `subject.substring` call introduces a StringLength(subject)
node, and the `subject.length` introduces a LoadField[length](subject)
node.

Consistently using StringLength operator everywhere enables
optimizations in TurboFan that had been blocked before here (besides
avoiding the redundant load operations).

Bug: v8:8015
Change-Id: I21c82bc418105b9933a9e60dde11c7b222dfcf4f
Reviewed-on: https://chromium-review.googlesource.com/1212942
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55710}
      60d3f89d