The problem was that synchronous Maglev OSR potentially caused
code deoptimization during compilation dependency finalization; this
led to a lazy deopt when returning from the call to
Runtime_CompileOptimizedOSRFromMaglev. However, a lazy deopt is
disallowed at this point, since a) Maglev doesn't support marking an opcode as both lazy- and eager deopt, and b) the JumpLoop opcode
is already marked as eager deopt since that's how OSR is implemented
under the hood. See also the comment in runtime-compiler.cc.
We fix this by changing synchronous Maglev-to-Turbofan OSR
behavior s.t. actual OSR compilation is triggered from Ignition
(and not from Maglev). In other words, when synchronous OSR is
requested:
1. trigger an eager deopt from Maglev to Ignition by returning a
non-null code object from Runtime_CompileOptimizedOSRFromMaglev.
2. Ignition handles the pending OSR compile request (through
osr_urgency).
This CL also reverts previous partial fixes:
This reverts commit 21969e8e24.
This reverts commit 6bcbcfed5c.
Bug: chromium:1394279,v8:13585,v8:7700
Change-Id: I3d64aa39575ad806ba2623102092176ca160ef0b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4110740
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84922}
If we'd do it before merging register values, we might drop a value from
a register that's needed by a phi.
Bug: v8:7700, chromium:1394036
Change-Id: I39be09d5ccf19ff70aaefc8865565f0d2169552c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4063692
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84576}
If a value is used after a try-block finishes, we need to make sure that
the catch-block can restore its value. Otherwise we'd accidentally drop
the value on register merge thinking we're in a liveness hole on the
merge after the catch (since the catch cleared all the registers). This
then breaks JumpLoops that need to restore the value in a specific
register.
Bug: v8:7700, chromium:1392061
Change-Id: I7255ccf9b36bf36583ad612882137b251c48caed
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4055111
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84483}
This reverts commit a63f9912b7.
Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64/50370/overview
Original change's description:
> [maglev] Spill nodes that we'd otherwise fail to merge
>
> This makes sure that catch-blocks don't accidentally drop values that
> are only in registers, which can happen if we throw in deferred throwing
> code (e.g., in ThrowReferenceErrorIfHole). At the latest we'll discover
> such values when trying to merge after the catch block, noticing we
> can't find the value through the catch-block. Unfortunately it's not
> trivial to figure out where that merge happens, so we just
> unconditionally spill the value.
>
> For liveness holes (as the comment previously mentioned) the value
> should already be dead and dropped on the merge. Running --maglev-stress
> etc shows that no code currently hits this path, except for the added
> test that shows the issue with catch blocks.
>
> Bug: chromium:1392061
> Change-Id: Ied0b1d4b430c9af2e7ae3dfc004ecb45037c5735
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4051605
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Auto-Submit: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#84448}
Bug: chromium:1392061
Change-Id: Iddbd7b19bc73e352dbd6867db990238f80adbdda
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4055504
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84455}
Function#call needs a function to call, so don't try to lower it to a
builtin call when there's no function.
Bug: v8:7700
Change-Id: I6705e2900731b2be2830231f8ab0dbfcdca5f594
Fixed: chromium:1392936
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4055680
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84451}
This makes sure that catch-blocks don't accidentally drop values that
are only in registers, which can happen if we throw in deferred throwing
code (e.g., in ThrowReferenceErrorIfHole). At the latest we'll discover
such values when trying to merge after the catch block, noticing we
can't find the value through the catch-block. Unfortunately it's not
trivial to figure out where that merge happens, so we just
unconditionally spill the value.
For liveness holes (as the comment previously mentioned) the value
should already be dead and dropped on the merge. Running --maglev-stress
etc shows that no code currently hits this path, except for the added
test that shows the issue with catch blocks.
Bug: chromium:1392061
Change-Id: Ied0b1d4b430c9af2e7ae3dfc004ecb45037c5735
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4051605
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84448}
The condition clearing the "known HeapObject" bit was wrong -- it was
checking whether the _map_ was a HeapObject (spoiler alert, it is), not
whether it was the map _of_ a HeapObject, i.e. not a HeapNumberMap which
returns true for Smis.
Bug: v8:7700
Change-Id: I5af4c1a662bb16bacdfcf178819d912332ecefd6
Fixed: chromium:1383712
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4023077
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84243}
The builtin inlining for FunctionPrototypeCall has to consider the case
where there is no new receiver to the call. It now does this by
considering the new call args to be kNullOrUndefined instead of kAny.
Drive-by cleanup of CallArguments to always consider the register count
and not the argument count, unifying the with/without receiver
correction for the list-of-regs and RegList cases.
Bug: v8:7700
Change-Id: I7e8cb7e9d654fdfcbb8add80e7a0a01a39d36504
Fixed: chromium:1381663, chromium:1381665
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4008638
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84092}
Copy the instanceof fast path from TurboFan, which emits an
'OrdinaryHasInstance' when there is no @@hasInstance symbol (which can
eventually become a constant true/false if we can look through the
prototype chain), and a direct call of @@hasInstance otherwise.
In particular, the call to @@hasInstance requires a continuation builtin
(to call ToBoolean), so add support for these too.
Bug: v8:7700
Change-Id: I14aee4346e98cd650f190b811cc7a733e33addae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3990844
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84012}
Our previous assumption that the receiver is immutable is incorrect in
constructors. Change the current logic (which never generates an
exception phi for receivers, but instead re-uses the parameter slot)
into forcing the receiver exception phi to be allocated (and spilled) in
the receiver parameter slot.
Bug: v8:7700
Change-Id: I1ba92b2e711dc0fcd7c818526b9c199cadcdd3bf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3948586
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83684}
Due to stack slot reuse, any of the moves that are part of the handler
trampoline may conflict and thus need parallel move resolution.
Materialisations (= calls to the NewHeapNumber builtin) add an addtl
complication since a) materialising moves can also be part of any
move conflict, b) the builtin call may clobber arbitrary registers,
and c) materialisation need a spot to store the NewHeapNumber result.
We resolve this by materialising into new temporary stack slots
before the main move sequence, and popping into the final target
locations after the main move sequence.
Bug: v8:7700
Change-Id: I1734faf189d02e38af07a817a9b647e2dce54f22
Fixed: chromium:1368046
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3921515
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83511}
Lazy deopts are always after calls, so force them to spill their inputs.
This would normally be the case anyway, except for deferred calls, which
don't tell the register allocator to spill like normal calls do.
This makes lazy deopt regalloc always spill its inputs and use their
spill slot, but unlike calls, this doesn't additionally clear the
register, so subsequent nodes can continue using the register cached
value without having to reload it.
As drive-bys, fix the Throw* opcodes to have the Throw property, and use
detail::DeepForEachInput in a couple of extra locations (including for
lazy deopts).
Bug: v8:7700
Change-Id: I89b04f17ca781d4f69ff0ed07566fa583aa677e6
Fixed: chromium:1364074
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3899009
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83293}
The kContextRegister can alias allocated registers - when setting it,
take care not to unintentionally clobber.
Bug: v8:7700
Change-Id: I0635d334fb14fa15540582a4873d4186fffa2199
Fixed: chromium:1363450
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3897634
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83212}
Temporaries and the allocated result register may alias, thus order is
important when setting the result value.
Fixed: TestUndetectable, LogicalNot, SetPendingMessage.
Drive-by: Pass Label::kNear in a few spots I passed by.
Bug: v8:7700
Change-Id: Ice3de1d1014ad05d8fa9fb18d967887386bfed0d
Fixed: chromium:1359723
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3898530
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83207}
Check whether the exception phi for the accumulator (i.e. the exception
message object) is dead, and don't assign rax to it if yes. Note that
maglev node liveness can differ from bytecode liveness, since the
bytecode accumulator could have been considered "live" just because of a
move to a (dead) register.
Bug: v8:7700
Change-Id: If1384284f6f55a565e2ae94e5e7a32455fdedb93
Fixed: chromium:1359382
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3892353
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83197}
Ignition remembers the correct context to restore when entering an
exception handler by moving the context to an interpreter register
when entering a try block, and restoring it from there when unwinding
the frame and entering the catch block.
Maglev code has to do the same by taking the context from the
appropriate register for the handler's frame state.
Bug: v8:7700
Change-Id: I294fcccc845c660b2289b6d7b40f49f1aa46283d
Fixed: chromium:1359928
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3892352
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83181}
