Added null check when printing the brand with --print-ast.
Bug: chromium:961507, chromium:961508
Original change's description:
> [class] implement private method declarations
>
> This patch implements the declarations of private methods, the access
> of private methods would be left to a future patch.
> When a private methods declaration is encountered, we now:
>
> - Create a brand symbol during class evaluation and store it in the
> context.
> - Create the closures for the private methods
> - Load the brand from the context and store it in the instance in the
> constructor.
>
> Design: https://docs.google.com/document/d/1T-Ql6HOIH2U_8YjWkwK2rTfywwb7b3Qe8d3jkz72KwA/edit#
>
> Bug: v8:8330
> Change-Id: I2d695cbdc8a7367ddc7620d627b318f779d36150
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1568708
> Commit-Queue: Joyee Cheung <joyee@igalia.com>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61387}
Change-Id: I3bf465f70c27914c9ec19f3f59ae018b28c9a866
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1605521
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61459}
This reverts commit b9191bd355.
Reason for revert: Clusterfuzz bugs
BUG=chromium:961507,chromium:961508
Original change's description:
> [class] implement private method declarations
>
> This patch implements the declarations of private methods, the access
> of private methods would be left to a future patch.
> When a private methods declaration is encountered, we now:
>
> - Create a brand symbol during class evaluation and store it in the
> context.
> - Create the closures for the private methods
> - Load the brand from the context and store it in the instance in the
> constructor.
>
> Design: https://docs.google.com/document/d/1T-Ql6HOIH2U_8YjWkwK2rTfywwb7b3Qe8d3jkz72KwA/edit#
>
> Bug: v8:8330
> Change-Id: I2d695cbdc8a7367ddc7620d627b318f779d36150
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1568708
> Commit-Queue: Joyee Cheung <joyee@igalia.com>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61387}
TBR=rmcilroy@chromium.org,gsathya@chromium.org,verwaest@chromium.org,joyee@igalia.com
Change-Id: I429bbe8af9f94598de132814aa2c3ab9fa69b986
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8330
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1605730
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61406}
Per 262, yield* for async generators is supposed to Await the input value.
#sec-generator-function-definitions-runtime-semantics-evaluation
Section 14.4.13, yield *, 7.c.iii.1
Bug: v8:9051
Change-Id: Ie1e829309fe78683a9ff0adf816208c2bf0bb524
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1600508
Commit-Queue: Joshua Litt <joshualitt@google.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61390}
This patch implements the declarations of private methods, the access
of private methods would be left to a future patch.
When a private methods declaration is encountered, we now:
- Create a brand symbol during class evaluation and store it in the
context.
- Create the closures for the private methods
- Load the brand from the context and store it in the instance in the
constructor.
Design: https://docs.google.com/document/d/1T-Ql6HOIH2U_8YjWkwK2rTfywwb7b3Qe8d3jkz72KwA/edit#
Bug: v8:8330
Change-Id: I2d695cbdc8a7367ddc7620d627b318f779d36150
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1568708
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61387}
We see crashes in the wild that we suspect are caused by these changes.
This is a manual revert because of conflicts.
Revert "[turbofan] Fix incorrect CheckNonEmptyString lowering."
This reverts commit b3b7011867.
Revert "[turbofan] Fix incorrect lowering of CheckNonEmptyString."
This reverts commit 5758209026.
Revert "[turbofan] Significantly improve ConsString creation performance."
This reverts commit d6a60a0ee1.
Bug: v8:9147
Change-Id: I262c21e5406a9c4c8ad0e0f995582c5802f0fa1e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1571613
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60919}
- Add a new ClassScope for block scopes created for classes.
- Add a VariableMap in the class scope for private name resolution,
and a separate UnresolvedList for private names that will be resolved
only using ClassScopes. These are stored in RareData and will only be
allocated when there are private name declaration or access in the
class.
Design: https://docs.google.com/document/d/1l-D70uaHzXU8QVgQZ3ACikb3FLO6LTAfQVdGDXsh5mw/edit?usp=sharing
TBR: hpayer@chromium.org
Bug: v8:8330
Bug: v8:7468
Change-Id: I78191fc075f7f195f6c56c959773c382346cce8a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1488271
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60726}
This change significantly improves the performance of string
concatenation in optimized code for the case where the resulting string
is represented as a ConsString. On the relevant test cases we go from
serializeNaive: 10762 ms.
serializeClever: 7813 ms.
serializeConcat: 10271 ms.
to
serializeNaive: 10278 ms.
serializeClever: 5533 ms.
serializeConcat: 10310 ms.
which represents a 30% improvement on the "clever" benchmark, which
tests specifically the ConsString creation performance.
This was accomplished via a couple of different steps, which are briefly
outlined here:
1. The empty_string gets its own map, so that we can easily recognize
and handle it appropriately in the TurboFan type system. This
allows us to express (and assert) that the inputs to NewConsString
are non-empty strings, making sure that TurboFan no longer creates
"crippled ConsStrings" with empty left or right hand sides.
2. Further split the existing String types in TurboFan to be able to
distinguish between OneByte and TwoByte strings on the type system
level. This allows us to avoid having to dynamically lookup the
resulting ConsString map in case of ConsString creation (i.e. when
we know that both input strings are OneByte strings or at least
one of the input strings is TwoByte).
3. We also introduced more finegrained feedback for the Add bytecode
in the interpreter, having it collect feedback about ConsStrings,
specifically ConsOneByteString and ConsTwoByteString. This feedback
can be used by TurboFan to only inline the relevant code for what
was seen so far. This allows us to remove the Octane/Splay specific
magic in JSTypedLowering to detect ConsString creation, and instead
purely rely on the feedback of what was seen so far (also making it
possible to change the semantics of NewConsString to be a low-level
operator, which is only introduced in SimplifiedLowering by looking
at the input types of StringConcat).
4. On top of the before mentioned type and interpreter changes we added
new operators CheckNonEmptyString, CheckNonEmptyOneByteString, and
CheckNonEmptyTwoByteString, which perform the appropriate (dynamic)
checks.
There are several more improvements that are possible based on this, but
since the change was already quite big, we decided not to put everything
into the first change, but do some follow up tweaks to the type system,
and builtin optimizations later.
Tbr: mstarzinger@chromium.org
Bug: v8:8834, v8:8931, v8:8939, v8:8951
Change-Id: Ia24e17c6048bf2b04df966d3cd441f0edda05c93
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Doc: https://bit.ly/fast-string-concatenation-in-javascript
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1499497
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60318}
This is a pre-work for allocating feedback vectors lazily. Feedback cells
are required to share the feedback vectors across the different closures
of the same function. Currently, they are held in the CreateClosureSlot
in the feedback vector. With lazy feedback vector allocation, we may not
have a feedback vector. However, we still need a place to store the
feedback cells, so if feedback vector is allocated in future it can still
be shared across closures.
Here is the detailed design doc:
https://docs.google.com/document/d/1m2PTNChrlJqw9MiwK_xEJfqbFHAgEHmgGqmIN49PaBY/edit
BUG=v8:8394
Change-Id: Ib406d862b2809b1293bfecdcfcf8dea3127cb1c7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1503753
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60147}
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/1493132
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Matt Gardner <magardn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#59958}
This is a reland of 35269f77f8
Switches on an expression that unconditionally throws would have all their
case statements dead, causing a DCHECK error in the SwitchBuilder. This
fixes up the DCHECK to allow dead labels.
Original change's description:
> [ignition] Skip binding dead labels
>
> BytecodeLabels for forward jumps may create a dead basic block if their
> corresponding jump was elided (due to it dead code elimination). We can
> avoid generating such dead basic blocks by skipping the label bind when
> no corresponding jump has been observed. This works because all jumps
> except JumpLoop are forward jumps, so we only have to special case one
> Bind for loop headers to bind unconditionally.
>
> Since Binds are now conditional on a jump existing, we can no longer rely
> on using Bind to get the current offset (e.g. at the beginning of a try
> block). Instead, we now expose the current offset in the bytecode array
> writer. Conveniently, this means that we can be a bit smarter about basic
> blocks around these statements.
>
> As a drive-by, remove the unused Bind(target,label) function.
>
> Bug: chromium:934166
> Change-Id: I532aa452fb083560d07b90da99caca0b1d082aa3
> Reviewed-on: https://chromium-review.googlesource.com/c/1488763
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59942}
TBR=rmcilroy@chromium.org
Bug: chromium:934166
Change-Id: If6eab4162106717ce64a2dc477000c6a76354cb4
Reviewed-on: https://chromium-review.googlesource.com/c/1494535
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59948}
This reverts commit 35269f77f8.
Reason for revert: Fuzzer unhappy: https://ci.chromium.org/p/v8/builders/ci/V8%20Fuzzer/29792
Original change's description:
> [ignition] Skip binding dead labels
>
> BytecodeLabels for forward jumps may create a dead basic block if their
> corresponding jump was elided (due to it dead code elimination). We can
> avoid generating such dead basic blocks by skipping the label bind when
> no corresponding jump has been observed. This works because all jumps
> except JumpLoop are forward jumps, so we only have to special case one
> Bind for loop headers to bind unconditionally.
>
> Since Binds are now conditional on a jump existing, we can no longer rely
> on using Bind to get the current offset (e.g. at the beginning of a try
> block). Instead, we now expose the current offset in the bytecode array
> writer. Conveniently, this means that we can be a bit smarter about basic
> blocks around these statements.
>
> As a drive-by, remove the unused Bind(target,label) function.
>
> Bug: chromium:934166
> Change-Id: I532aa452fb083560d07b90da99caca0b1d082aa3
> Reviewed-on: https://chromium-review.googlesource.com/c/1488763
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59942}
TBR=rmcilroy@chromium.org,leszeks@chromium.org
Change-Id: I8118e54e0afa5e08b0a0a874c952f8a01f1c3242
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:934166
Reviewed-on: https://chromium-review.googlesource.com/c/1494534
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59947}
BytecodeLabels for forward jumps may create a dead basic block if their
corresponding jump was elided (due to it dead code elimination). We can
avoid generating such dead basic blocks by skipping the label bind when
no corresponding jump has been observed. This works because all jumps
except JumpLoop are forward jumps, so we only have to special case one
Bind for loop headers to bind unconditionally.
Since Binds are now conditional on a jump existing, we can no longer rely
on using Bind to get the current offset (e.g. at the beginning of a try
block). Instead, we now expose the current offset in the bytecode array
writer. Conveniently, this means that we can be a bit smarter about basic
blocks around these statements.
As a drive-by, remove the unused Bind(target,label) function.
Bug: chromium:934166
Change-Id: I532aa452fb083560d07b90da99caca0b1d082aa3
Reviewed-on: https://chromium-review.googlesource.com/c/1488763
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59942}
This reverts commit 32fc0acfef.
Reason for revert:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064/30270
layout test breakage:
https://test-results.appspot.com/data/layout_results/V8-Blink_Linux_64/30270/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html
There is a dead node arriving in representation selection, which might indicate that the problem is not in this CL, but that this CL stirs up the node soup in such a way that dead code elimination gets confused.
Original change's description:
> Optimize `in` operator
>
> This change implements optimizations for the `in` operator for packed array
> elements and object properties. It adds a new feedback slot kind and an IC
> path similar to KeyedLoadIC for handling the lookups. TurboFan uses the
> feedback to optimize based on the maps and keys.
>
> For more details see:
> https://docs.google.com/document/d/1tIfzywY8AeNVcy_sen-5Xev21MeZwjcU8QhSdzHvXig
>
> This can provide 10x performance improvements of on loops of the form:
>
> for (let i = 0; i < ary.length; ++i) {
> if (i in ary) {
> ...
> }
> }
>
>
> Bug: v8:8733
> Change-Id: I766bf865a547a059e5bce5399bb6112e5d9a85c8
> Reviewed-on: https://chromium-review.googlesource.com/c/1432598
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Matt Gardner <magardn@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#59843}
TBR=ulan@chromium.org,rmcilroy@chromium.org,jkummerow@chromium.org,jarin@chromium.org,ishell@chromium.org,bmeurer@chromium.org,verwaest@chromium.org,magardn@microsoft.com
Change-Id: Ib2db974e5bed4c4a2b6b450f796bdc4b0b8fd562
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8733
Reviewed-on: https://chromium-review.googlesource.com/c/1488761
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59857}
This change implements optimizations for the `in` operator for packed array
elements and object properties. It adds a new feedback slot kind and an IC
path similar to KeyedLoadIC for handling the lookups. TurboFan uses the
feedback to optimize based on the maps and keys.
For more details see:
https://docs.google.com/document/d/1tIfzywY8AeNVcy_sen-5Xev21MeZwjcU8QhSdzHvXig
This can provide 10x performance improvements of on loops of the form:
for (let i = 0; i < ary.length; ++i) {
if (i in ary) {
...
}
}
Bug: v8:8733
Change-Id: I766bf865a547a059e5bce5399bb6112e5d9a85c8
Reviewed-on: https://chromium-review.googlesource.com/c/1432598
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Matt Gardner <magardn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#59843}
Vars without initialisers don't need to allocate a VariableProxy, as the
proxy expression is not really needed for anything. So, we can special
case declaration parsing to look ahead for a '=' (plus a few other
cases), and skip the variable proxy allocation if it isn't there.
As a side-effect, variables that are only declared but never used are
no longer marked is_used, and thus not allocated. This saves on
generating dead code.
Change-Id: Ie4f04c6b5c1138df4c2e17acf1f0150459b3b571
Reviewed-on: https://chromium-review.googlesource.com/c/1434376
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59129}
SetProperty now infers the language mode from the closure and the context
So we no longer have to pass around the language mode. Cleanup by
removing the parameter where it is no longer needed.
Bug: v8:8580
Change-Id: I89452b5a762eb48a911f158d22c7bfa9e3bb1be4
Reviewed-on: https://chromium-review.googlesource.com/c/1421840
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59089}
Declare Variables with a name and position, rather than by passing
through a VariableProxy. This allows us to not create dummy proxies
for things like function declarations, and allows us to consider those
declarations unused.
As a side-effect, we also have to check if a variable is unused in the
bytecode generator (as it will no longer be allocated), and we end up
skip generating code/SFIs for dead variables/functions.
Change-Id: I4c2c872473f23e124f9456b4b92f87159658f8e0
Reviewed-on: https://chromium-review.googlesource.com/c/1414916
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59088}
V8 implements "delete this" as "LdaTrue", but an error needs to be thrown
if done in a constructor before calling super. ThrowIfHole checks the
accumulator, so we need to load 'this' into the accumulator. The check is
inserted by the load since it has HoleCheckMode::kRequired
Bug: https://bugs.chromium.org/p/v8/issues/detail?id=6711
Change-Id: I9f2ce4439505cec4327d88d1195898782edea721
Reviewed-on: https://chromium-review.googlesource.com/c/1419084
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Matt Gardner <magardn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#59007}
The decision as to whether to optimize an IIFE as oneshot depends on
whether it's outer scope is the script scope. During lazy compile, we
might have discarded scopes which don't need a context between the IIFE
and the script scope, which means we might treat an IIFE as oneshot,
even though initial eager compile treated it as non-oneshot. Both
bytecode flushing and lazy source positions rely on us generating the
same bytecode during lazy compile as eager compile, so we move the
decision into the parser where it happens once and is then stored in
the SFI for any future lazy compiles.
BUG=v8:8395,v8:8510
Change-Id: I88f1e74ad95d47a2636c393ceb1318d7d610055d
Reviewed-on: https://chromium-review.googlesource.com/c/1421841
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58996}
Use variable tracking from ExpressionScopes rather than the PatternRewriter and
PreParserExpression::variables_ to declare variables.
We only figure out that variables are non-simple parameters once we see the
first non-simple parameter. This still uses the pattern rewriter to make
variables non-simple (kLet instead of kVar).
Change-Id: I4a4ee4852d667c26806bb24896722cfea3e093f2
Reviewed-on: https://chromium-review.googlesource.com/c/1417630
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58954}
A sloppy function in a block scope implicitily creates a var in the outer
declaration scope if it's not blocked. The assignment created reads the local
lexical declaration for the function. The reference introduced automatically
takes part in NeedsHoleCheck, requiring the reference to have a valid position.
Since the assignment will happen after the local declaration, we give the
end_position() of the closure as the position of the reference, so hole checks
can be omitted.
Bug: chromium:917755
Change-Id: Iee0e042b2463f97f05075f9eec09dac8c6eaf539
Reviewed-on: https://chromium-review.googlesource.com/c/1408991
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58823}
This removes the iteration protocol from the parser entirely, and opens
up future possibilities for more bytecodes implementing the various
functions of the protocol.
Change-Id: I316b8a92434d3b5f47927408a235ddaecd65d5bb
Reviewed-on: https://chromium-review.googlesource.com/c/1403125
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58795}
For desrtucturing assignments from null/undefined, we throw an error
that references the destructuring object literal's property name, e.g.
for
var { x } = null;
we report that we cannot destructure 'x' from null.
Rather than calculating this property during bytecode generation (and
including it in the bytecode as an argument to the type error
constructor), we can calculate it at exception throwing time, by
re-parsing the source in a similar way to the existing call site
rendering.
This slightly decreases bytecode size and slightly decreases the amount
of work the bytecode compiler needs to do. In the future, it could also
allow us to give more detailed error messages, as we now have access to
the entire AST and are on the slow path anyway.
Bug: v8:6499
Change-Id: Icdbd4667db548b4e5e62ef97797a3771b5c1bf72
Reviewed-on: https://chromium-review.googlesource.com/c/1396080
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58706}
The 'done' setting dance in BuildFillArrayWithIterator turned out to
not be useful, as the StoreInArrayLiteral call could not ever throw an
exception. Since iterator exceptions count as done, we are guarnteed to
be done as soon as we enter the loop.
Change-Id: Ibe2ba1fcbe383bfcfedb185169890b6931cc7884
Reviewed-on: https://chromium-review.googlesource.com/c/1402792
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58695}
The IteratorClose spec specifies that exceptions in
%GetMethod(iterator.return) are not suppressed by exceptions in the
given continuation (body of a loop, assignments in destructuring),
while exceptions in the execution of iterator.return() are.
This means that we have to split out the property access + a typeof
check to be outside the try-catch, and keep the call inside of it.
The non-split version is only for cases when there is no 'throws'
continuation (as is the case for yield* calling IteratorClose), so
the existing BuildIteratorClose can be renamed to reflect this.
Change-Id: Id71aea4fddd6ffb986bd9aaa09d29615a8800f71
Reviewed-on: https://chromium-review.googlesource.com/c/1402789
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58694}
Emit a single destructuring assignment for destructuring declarations,
which can be desugared by the bytecode generator. This allows us to
remove destructuring desugaring from the parser (specifically, the
pattern rewriter) entirely.
The pattern "rewriter" is now only responsible for walking the
destructuring pattern to declare variables, mark them assigned, and
potentially rewrite scopes for the edge case of parameters with a sloppy
eval.
Note that since the rewriter is no longer rewriting, we have to flip the
VariableProxy copying logic for var re-lookup, so that we now pass the
new VariableProxy to the variable declaration and leave the original
unresolved (rather than passing the original through and rewriting to a
new unresolved VariableProxy).
This change does have some effect on breakpoint locations, due to some
of the available information changing between the parser and bytecode
generator, however the new locations appear to be more consistent
between assignments and declarations.
Change-Id: I3a58dd0a387d2bfb8e5e9e22dde0acc5f440cb82
Reviewed-on: https://chromium-review.googlesource.com/c/1382462
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58670}
This patch sets the name slot of the private name symbols for
private fields and display the names in error messages of invalid
private field accesses.
TBR: adamk@chromium.org
Bug: v8:8144
Change-Id: Id34c468e2bddd1c3001517b4d447c7497402df76
Reviewed-on: https://chromium-review.googlesource.com/c/1374332
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/master@{#58601}
Instead of de-sugaring destructuring assignment in the parser (using the
pattern rewriter), pass the Object/ArrayLiterals through to the bytecode
generator, which can desugar them in-place.
This allows us to decrease the amount of AST node creation, and improve
the generated bytecode using domain-specific knowledge. As a side effect
we partially fix an old execution ordering spec bug.
Currently only implemented for assignments, not declarations, as the
latter has some additional complexity.
Bug: v8:4951
Change-Id: I3d69d232bea2968ef20df68a74014d9e05808cfe
Reviewed-on: https://chromium-review.googlesource.com/c/1375660
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58512}
Currently, the runtime IC functions deduce the IC kind and the language
mode from the feedback slot kind. To support feedback free execution
(for V8 lite mode and lazy allocation of feedback vectors) we need to
infer the IC kind even when feedback vectors are not present.
To be able to infer the language mode without feedback vectors, this cl
forces context allocation in cases where we raise the language mode in
the middle of a function. The language mode is the stricter of the
language mode on the SFI and the language mode of the current context.
This cl updates the bytecode handlers to check for valid feedback vectors
and to call into runtime if the feedback vector is not allocated. It also
adds new runtime functions to be able to infer the IC kind when there is no
feedback vector. Most of the builtins and handlers remain unchanged because
they are only used when feedback vector is present.
Bug: v8:8394
Change-Id: I1f77740c0d68ddaa0de076597f5f6bcb2e966d70
Reviewed-on: https://chromium-review.googlesource.com/c/1358516
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58191}
This patch changes the output from:
function fn() {
^
SyntaxError: Unexpected end of input
to:
function fn() {
^
SyntaxError: missing '}' after function body
Bug: v8:6513, v8:7321
Change-Id: I4ca8a40fa0be246da2a3ff776b3fb3c87b4ba4e0
Also-By: gsathya@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1367448
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58116}
Now that `do` expressions have been removed [1], we should remove the
corresponding golden file as well.
[1]: 58988c6c63
Change-Id: I4a6908d4d53b2bce1d9f2d616c672b4caf15adac
Reviewed-on: https://chromium-review.googlesource.com/c/1365283
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58084}
As opposed to the register.
For subtle reasons, this fixes a deoptimizer bug with handling return
values in lazy deopt. Since the return values can now only overwrite
the accumulator, there is no danger of overwriting a captured object
that might be later used (since there is no "later").
Bug: chromium:902608
Change-Id: I3a7a10bb1c7a6f4303a01d60f80680afcb7bc942
Reviewed-on: https://chromium-review.googlesource.com/c/1325901
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57349}
Rename variables and flag names so that the classes can be reused
by private methods implementation.
In particular:
Rename "fields" to "members" in the initializer so that we can
initialize both fields and private methods/accessors there,
for example:
instance_fields_initializer -> instance_members_initializer
InitializeClassFieldsStatement -> InitializeClassMembersStatement
Rename "private field" to "private name" for the private symbols
used to implement private fields so that we can use them to
store private methods/accessors later as well, for example:
private_field_name_var -> private_name_var
NewPrivateFieldSymbol -> NewPrivateNameSymbol
The follow-on is in
https://chromium-review.googlesource.com/c/v8/v8/+/1301018
The design doc is in
https://docs.google.com/document/d/1T-Ql6HOIH2U_8YjWkwK2rTfywwb7b3Qe8d3jkz72KwA/edit?usp=sharing
Bug: v8:8330
Change-Id: I1cdca8def711da879b6e4d67c5ff0a5a4a36abbe
Reviewed-on: https://chromium-review.googlesource.com/c/1312597
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/master@{#57289}
Adding test to confirm that no one-shot optimizations are not done
for functions enclosed in parentheses but not immediately invoked
in an assignment.
Bug: v8:8072
Change-Id: I282132a7cc570b59290f2ec314462be060d48e5a
Reviewed-on: https://chromium-review.googlesource.com/c/1238576
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56763}
Change the way we start collecting async stack traces by storing the
current microtask as a root instead of trying to make sense of the
last frame we see. This makes it possible to use the zero cost async
stack traces in Node.js as well (where the last JavaScript frame we
see is not the actual async function, but some frame related to the
main event loop usually).
In addition to the benefit that it now works with Node.js, we can also
extend the new machinery to look through (almost arbitrary) promise
chains. For example this code snippet
```js
(async function() {
await Promise.resolve().then(() =>
console.log(new Error().stack));
})();
```
can be made to also show the async function frame, even though at the
point where the stack trace is collected we don't have any async
function on the stack. But instead there's a PromiseReactionJobTask
as "current microtask", and we can dig into the chained promise to
see where the async execution is going to continue and eventually
find the await promise in the chain.
This also removes the removes the need to allocate `.generator_object`
specially during scope resolution.
Bug: v8:7522
Ref: nodejs/node#11865
Tbr: ulan@chromium.org
Design-Document: bit.ly/v8-zero-cost-async-stack-traces
Change-Id: Ib96cb17c2f75cce083a24e5ba2bbb7914e20d203
Reviewed-on: https://chromium-review.googlesource.com/c/1277505
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56590}
This JSAsyncFunctionObject represents the implicit generator object
inside of async functions, and also holds the outer promise for the
async functions. This in turn allows us to get rid of the .promise
in the Parser / BytecodeGenerator completely, and will make it
possible to build zero-cost async stack traces independent of the
concrete synchronous part of the stack frame (which currently breaks
in Node.js).
In the bytecode all the async function operations now take this new
JSAsyncFunctionObject instead of passing both the .generator_object
and the .promise, which further simplifies and shrinks the bytecode.
It also reduces the size of async function frames, potentially making
the suspend/resume cheaper.
This also changes `await` to use intrinsics instead of calling to
special JSFunctions on the native context, and thus reduces the size of
the native contexts.
Drive-by-fix: Introduce a dedicated JSCreateAsyncFunctionObject operator
to TurboFan.
Bug: v8:7253, v8:7522
Change-Id: I2305302285156aa1f71328ecac70377abdd92c80
Ref: nodejs/node#11865
Design-Document: http://bit.ly/v8-zero-cost-async-stack-traces
Reviewed-on: https://chromium-review.googlesource.com/c/1273049
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56554}
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: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56502}
The Parser desugaring didn't use the AsyncReturnStatement consistently
to return from async functions (aka resolve the .promise with the return
value and return the .promise from the async function). Instead the
Parser essentially had a copy of the BytecodeGenerator functionality.
This change unifies the handling of returns from async functions.
Bug: v8:7522, v8:8238
Change-Id: Ib00a60aee30d541b84835d9cc83e9937b7a39e26
Reviewed-on: https://chromium-review.googlesource.com/c/1269036
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56453}
This forces .generator_object variable to stack slot 0 for async
generator functions so that the stack trace construction logic
can extract the JSAsyncGeneratorObject appropriately.
Bug: v8:7522
Change-Id: I37b52836bb512bcf5cd7e10e1738c8e7895b06ea
Ref: nodejs/node#11865
Design-Document: http://bit.ly/v8-zero-cost-async-stack-traces
Reviewed-on: https://chromium-review.googlesource.com/c/1264556
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56415}
This introduces a new flag --async-stack-traces, which enables zero-cost
async stack traces. This enriches the non-standard Error.stack property
with async stack frames computed from walking up the promise chains and
collecting all the await suspension points along the way. In Error.stack
these async frames are marked with "async" to make it possible to
distinguish them from regular frames, for example:
```
Error: Some error message
at bar (<anonymous>)
at async foo (<anonymous>)
```
It's zero-cost because no additional information is collected during the
execution of the program, but only the information already present in the
promise chains is used to reconstruct an approximation of the async stack
in case of an exception. But this approximation is limited to suspension
points at await's in async functions. This depends on a recent ECMAScript
specification change, flagged behind --harmony-await-optimization and
implied the --async-stack-traces flag. Without this change there's no
way to get from the outer promise of an async function to the rest of
the promise chain, since the link is broken by the indirection introduced
by await.
For async functions the special outer promise, named .promise in the
Parser desugaring, is now forcible allocated to stack slot 0 during
scope resolution, to make it accessible to the stack frame construction
logic. Note that this first prototype doesn't yet work fully support
async generators and might have other limitations.
Bug: v8:7522
Ref: nodejs/node#11865
Change-Id: I0cc8e3cdfe45dab56d3d506be2d25907409b01a9
Design-Document: http://bit.ly/v8-zero-cost-async-stack-traces
Reviewed-on: https://chromium-review.googlesource.com/c/1256762
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56363}
This is a reland of eccf186749
Original change's description:
> [interpreter] Separate bytecodes for one-shot property loads and stores
>
> Create LdaNamedPropertyNoFeedback and StaNamedPropertyNoFeedback
> for one-shot property loads and stores. This CL replaces the runtime
> calls with new bytecodes for named property load stores in one-shot code.
> the runtime calls needed extra set of consecutive registers and
> additional move instructions. This increased the size of
> bytecode-array and possibly extended the life time of objects.
> By replacing them with NoFeedback bytecodes we avoid these issues.
>
> Bug: v8:8072
> Change-Id: I20a38a5ce9940026171d870d354787fe0b7c5a6f
> Reviewed-on: https://chromium-review.googlesource.com/1196725
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Chandan Reddy <chandanreddy@google.com>
> Cr-Commit-Position: refs/heads/master@{#56211}
Bug: v8:8072
Change-Id: Ie8e52b37daf35c7bc08bb910d7b15a9b783354e4
Reviewed-on: https://chromium-review.googlesource.com/1245742
Commit-Queue: Chandan Reddy <chandanreddy@google.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56266}
This reverts commit eccf186749.
Reason for revert: Speculative revert because it seems to introduce a pretty stable flake on gc stress tests, see https://bugs.chromium.org/p/v8/issues/detail?id=8229
Original change's description:
> [interpreter] Separate bytecodes for one-shot property loads and stores
>
> Create LdaNamedPropertyNoFeedback and StaNamedPropertyNoFeedback
> for one-shot property loads and stores. This CL replaces the runtime
> calls with new bytecodes for named property load stores in one-shot code.
> the runtime calls needed extra set of consecutive registers and
> additional move instructions. This increased the size of
> bytecode-array and possibly extended the life time of objects.
> By replacing them with NoFeedback bytecodes we avoid these issues.
>
> Bug: v8:8072
> Change-Id: I20a38a5ce9940026171d870d354787fe0b7c5a6f
> Reviewed-on: https://chromium-review.googlesource.com/1196725
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Chandan Reddy <chandanreddy@google.com>
> Cr-Commit-Position: refs/heads/master@{#56211}
TBR=rmcilroy@chromium.org,yangguo@chromium.org,jarin@chromium.org,neis@chromium.org,cbruni@chromium.org,chandanreddy@google.com
Change-Id: I445db58e6d4c275b434fabad5fad775bf259033f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8072
Reviewed-on: https://chromium-review.googlesource.com/1245421
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56232}
