The tricky part here is to take away one register from register
allocation for the mask. The only problem is with calls that need
an input operand to be passed in the poison register. For such calls,
we change the register constraint in the instruction selector
to pass the value in whatever place the register allocator sees fit.
During code generation, we then copy the value from that place
to the poison register. By that time, the mask is not necessary
(once we bake the mask into the target, it should be done before
this move).
For the branches, the mask update does not use cmov (unlike x64)
because cmov does not take an immediate and we do not have
a scratch register. Instead we use bit-twiddling tricks
(suggested by @tebbi). For example, here is the code for masking
register update after a bailout on non-zero:
jnz deopt_bailout ;; Bailout branch
setnz bl ;; These three instructions update the mask
add ebx, 255
sar ebx, 31
(On x64, the sequence is:
jnz deopt_bailout
mov r10, 0 ;; We have a scratch register for zero
cmovnz r9, r10 ;; Set to zero if we execute this branch
;; in branch mis-speculation
)
This CL also fixes a bug in register configuration, where we used
to wrongly restrict the array of register name.
Change-Id: I5fceff2faf8bdc527d9934afc284b749574ab69e
Bug: chromium:798964
Reviewed-on: https://chromium-review.googlesource.com/946251
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51798}
x86, arm, arm64: no change in behavior
mips, mips64: disasm-mips(64).cc grows an UNREACHABLE that's
maybe optimistic (but if it's not true, then that
looks like a current unintentional fallthrough at
that spot)
test-js-typed-lowering.cc: looks like a clear bug, but test-only code
Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/911731 which
did this for x64.
Doesn't turn on the warning yet.
Bug: chromium:812686
Change-Id: I7dd79c9885c90f41dd7e3a595256a954ab0ae643
Reviewed-on: https://chromium-review.googlesource.com/923528
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51437}
This introduces masking of loads with speculation bit during code generation.
At the moment, this is done only for x64 optimized code, under the
--branch-load-poisoning flag.
Overview of changes:
- new register configuration configuration with one register reserved for
the speculation poison/mask (kSpeculationPoisonRegister).
- in codegen, we introduce an update to the poison register at the starts
of all successors of branches (and deopts) that are marked as safety
branches (deopts).
- in memory optimizer, we lower all field and element loads to PoisonedLoads.
- poisoned loads are then masked in codegen with the poison register.
* only integer loads are masked at the moment.
Bug: chromium:798964
Change-Id: Ie51fdbde578fc289dff029794f3cfe8eaf33e1ef
Reviewed-on: https://chromium-review.googlesource.com/901625
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51272}
This is a purely cosmetic change. Rename all local variables and
parameters of type CallDescriptor* to "call_descriptor".
For locals that are now named "call_descriptor", use auto upon
initialization, following the Google style guide
(https://google.github.io/styleguide/cppguide.html#auto).
Note: fields in structs and classes were not renamed in this CL.
R=clemensh@chromium.org,mstarzinger@chromium.org,jarin@chromium.org
Change-Id: Ic6f7afdba12f7b97741b098a9d0e0f58c41c587e
Reviewed-on: https://chromium-review.googlesource.com/909866
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51223}
Port 0761b55d21
Original Commit Message:
"Extend the code-generator tests to cover AssembleMove and AssembleSwap with
Simd128 registers and stack slots, for targets that support them.
For this to work however, we need support for passing Simd128 stack parameters
in TurboFan which this patch implements for Arm and x86. PPC and S390 both do
not support the Simd128 representation and it appears MIPS and MIPS64's
implementation of AssembleMove and AssembleSwap do not support it either.
As per the design of the tests, the set of values to perform moves on are
represented in a FixedArray of Smis (for kTagged) and HeapNumbers (for kFloat32
and kFloat64). They are converted to raw values for the moves to be performed
on, to be then converted back into a FixedArray. For the kSimd128
representation, we represent values as a FixedArray of 4 Smis, each representing
a lane. They are converted to a raw Simd128 vector using the `I32x4ReplaceLane`
and `I32x4ExtractLane` operations.
Finally, these tests need Simd128 variables mixed with the CodeStubAssembler
which is not a use-case officially supported. And as a result, the `RecordWrite`
stub does not guarantee to preserve Simd128 registers. To get around this, we
have to be careful to skip write barriers when dealing with Simd128 parameters
inside the "teardown" function, and we've had to move all allocations to the
"setup" function.
Thanks to this, we are able to catch bugs such as this one
https://bugs.chromium.org/p/v8/issues/detail?id=6843."
Change-Id: If867dedf4a2c72cb75c58effda93e3eec432fd67
Reviewed-on: https://chromium-review.googlesource.com/906469
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Ivica Bogosavljevic <ivica.bogosavljevic@mips.com>
Cr-Commit-Position: refs/heads/master@{#51142}
Extend the code-generator tests to cover AssembleMove and AssembleSwap with
Simd128 registers and stack slots, for targets that support them.
For this to work however, we need support for passing Simd128 stack parameters
in TurboFan which this patch implements for Arm and x86. PPC and S390 both do
not support the Simd128 representation and it appears MIPS and MIPS64's
implementation of AssembleMove and AssembleSwap do not support it either.
As per the design of the tests, the set of values to perform moves on are
represented in a FixedArray of Smis (for kTagged) and HeapNumbers (for kFloat32
and kFloat64). They are converted to raw values for the moves to be performed
on, to be then converted back into a FixedArray. For the kSimd128
representation, we represent values as a FixedArray of 4 Smis, each representing
a lane. They are converted to a raw Simd128 vector using the `I32x4ReplaceLane`
and `I32x4ExtractLane` operations.
Finally, these tests need Simd128 variables mixed with the CodeStubAssembler
which is not a use-case officially supported. And as a result, the `RecordWrite`
stub does not guarantee to preserve Simd128 registers. To get around this, we
have to be careful to skip write barriers when dealing with Simd128 parameters
inside the "teardown" function, and we've had to move all allocations to the
"setup" function.
Thanks to this, we are able to catch bugs such as this one
https://bugs.chromium.org/p/v8/issues/detail?id=6843.
Bug: v8:6848
Change-Id: I8787d6339cdbfcd9356c5e8995925f0b45c562fa
Reviewed-on: https://chromium-review.googlesource.com/728599
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50326}
This patch updates the instruction selector and code generator to pad arguments
for arm64 and drop an even number of slots when dropping the arguments. It also
updates the builtins that handle arguments. These changes need to be made at
the same time.
It also adds some tests for forwarding varargs, as this was affected by the
builtin changes and the existing tests did not catch all issues.
Bug: v8:6644
Change-Id: I81318d1d1c9ab2568f84f2bb868d2a2d4cb56053
Reviewed-on: https://chromium-review.googlesource.com/829933
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50259}
This is to avoid calling AssembleTailCallBeforeGap and AssembleTailCallAfterGap
directly where possible (so making the tests less dependent on the code generator
interface when we're not directly testing it). It also makes sure that the
instruction we pass to AssembleTailCallBeforeGap and AssembleTailCallAfterGap is
indeed a tail call, with the immediate argument that specifies the stack delta.
This is to prepare for padding arguments for arm64 JSSP removal. We will need to
store padding in AssembleTailCallAfterGap, which will need the information from
a TailCall instruction.
Bug: v8:6644
Change-Id: Ia5485412a4244c7b2a133aa0541b9f8285680de4
Reviewed-on: https://chromium-review.googlesource.com/806117
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Cr-Commit-Position: refs/heads/master@{#49833}
Removes Isolate from compilation info and instead threads isolate through
function calls. This ensures that we can't access the isolate from
background thread compilations.
BUG=v8:5203
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I9a4e1cd67c4736e36f609360b996fb55166a1c50
Reviewed-on: https://chromium-review.googlesource.com/751745
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49386}
The motivation for this is that it greatly reduces the RelocInfo size.
This also results in a small improvement in compile time.
Note: This CL was based on https://codereview.chromium.org/2651833003,
and basically reverts that CL (but handles code changes and some
minor bugs in previous code).
Bug: chromium:772780
Change-Id: I55dd48d3bddd4b3d1c8eec13791b3ee4c485c604
Reviewed-on: https://chromium-review.googlesource.com/730649
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Karl Schimpf <kschimpf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48947}
Even static constant fields need to have definitions outside of the
class scope if a reference to them is passed.
This CL fixes link errors which occured on an independent CL
(https://crrev.com/c/730716).
Drive-by: Make the fields constexpr.
R=mstarzinger@chromium.org
Change-Id: Iff5dd1f3d41ddfba0c20531dbecd63c1d4c670e8
Reviewed-on: https://chromium-review.googlesource.com/732114
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48816}
Introduce new `SimulateMoves` and `SimulateSwaps` methods which take an initial
"state" as a FixedArray and perform a given list of moves on it. They give us
what the result of testing the CodeGenerator's AssembleMove and AssembleSwap
should be.
This way, we can now compare the results of running parallel moves with a
reference simulation.
Bug: v8:6848
Change-Id: I228f4310f32d2a82e0744afaff183e2c7ac08cb7
Reviewed-on: https://chromium-review.googlesource.com/723222
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48656}
This patch is a first step towards target independent tests for the
CodeGenerator's AssembleMove and AssembleSwap methods.
The tests on top of which this builds would only make sure that no assertions
were triggered while generating moves, and that the hardware is happy executing
them. We want to do more and check that the generated code performs correctly.
In a nutshell, this introduces a facility that can do the following:
- Setup an environment with registers and stack slots initialised with random
values.
- Perform a list of randomly generated moves and/or swaps on those.
- Return the resulting environment.
This is a first step and therefore is lacking a few things which will be
implemented as follow-ups:
- Support for kSimd128 moves and swaps.
- Support large offsets for stack moves, as well as positive and negative.
- Compare the resulting environment against the result of a reference
simulation.
For more background information, see this design document:
https://docs.google.com/document/d/1KpioxCmtiB_9RaPaRidZPVtKlZ2BaNKGPYUjKFihhK0
Bug: v8:6848
Change-Id: Ie7dc837f4444df010ab58c64b722d40ee5d2af72
Reviewed-on: https://chromium-review.googlesource.com/677398
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Cr-Commit-Position: refs/heads/master@{#48459}
Use the (D)CHECK_{EQ,NE,GT,...} macros instead of (D)CHECK with an
embedded comparison. This gives better error messages and also does the
right comparison for signed/unsigned mismatches.
This will allow us to reenable the readability/check cpplint check.
R=jarin@chromium.org
Bug: v8:6837
Change-Id: I712580c2a4326e06ee3d6d0eb4ff8c7d24f5fdb9
Reviewed-on: https://chromium-review.googlesource.com/671227
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48135}
This is a reland of c6b153fd69
Original change's description:
> [cctest] Add fuzz tests for generating parallel moves.
>
> These new tests are somewhat similar to the existing gap resolver tests except
> we use the code generator and eventually run the generated code. The main idea
> is to cover cases that are difficult to hit, such as move from/to slots which
> are out of range of loads and stores, but may happen nonetheless.
>
> At this time, the tests only make sure the code generator actually generated
> some code, and that this code runs. In the future, it would be great to also
> check that the moves were actually performed.
>
> Bug: v8:6553
> Change-Id: I089a25fa05b3a20649658bb8952926ab11f91d68
> Reviewed-on: https://chromium-review.googlesource.com/574850
> Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47733}
Bug: v8:6553
Change-Id: Ia3eac9d7e6a23e2f6fea839b71d460cb7ad6ff6e
Reviewed-on: https://chromium-review.googlesource.com/645868
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Cr-Commit-Position: refs/heads/master@{#48115}
This reverts commit c6b153fd69.
Reason for revert: Doesn't compile on the tree.
Original change's description:
> [cctest] Add fuzz tests for generating parallel moves.
>
> These new tests are somewhat similar to the existing gap resolver tests except
> we use the code generator and eventually run the generated code. The main idea
> is to cover cases that are difficult to hit, such as move from/to slots which
> are out of range of loads and stores, but may happen nonetheless.
>
> At this time, the tests only make sure the code generator actually generated
> some code, and that this code runs. In the future, it would be great to also
> check that the moves were actually performed.
>
> Bug: v8:6553
> Change-Id: I089a25fa05b3a20649658bb8952926ab11f91d68
> Reviewed-on: https://chromium-review.googlesource.com/574850
> Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47733}
TBR=bbudge@chromium.org,danno@chromium.org,jarin@chromium.org,pierre.langlois@arm.com,bmeurer@chromium.org
Change-Id: I875ab38e039fdbf58b8f08658c391147d2ec01fa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6553
Reviewed-on: https://chromium-review.googlesource.com/645446
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47734}
These new tests are somewhat similar to the existing gap resolver tests except
we use the code generator and eventually run the generated code. The main idea
is to cover cases that are difficult to hit, such as move from/to slots which
are out of range of loads and stores, but may happen nonetheless.
At this time, the tests only make sure the code generator actually generated
some code, and that this code runs. In the future, it would be great to also
check that the moves were actually performed.
Bug: v8:6553
Change-Id: I089a25fa05b3a20649658bb8952926ab11f91d68
Reviewed-on: https://chromium-review.googlesource.com/574850
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47733}
Code size in snapshot can be reduced ~41KB
Contributed by kanghua.yu@intel.com
Bug: None
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ib73af39fe97cd38728affea40c593236f15bf6e5
Reviewed-on: https://chromium-review.googlesource.com/588751
Commit-Queue: Pan Deng <pan.deng@intel.com>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47531}
This removes the obsolete {Crankshaft} factory method as it returns the
same configuration as the {Turbofan} factory by now. We now consistently
use {RegisterConfiguration::Default} everywhere.
R=jkummerow@chromium.org
BUG=v8:6408
Change-Id: I6be25774aa6714ef4dc1ef6856bb6dbc95593a29
Reviewed-on: https://chromium-review.googlesource.com/597858
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47109}
This is a reland of a72b2f88a8
Original change's description:
> [arm] Restrict grouping pushes before a TailCall to registers only
>
> We optimize parallel moves performed before a TailCall by grouping adjacent
> pushes. This way, we may use a single instruction to push multiple registers at
> once. However, we also have support for pushing immediates and stack slots for
> which the benefit is questionnable therefore this patch removes support for
> them.
>
> Concerning immediate pushes, it looks like a mistake since we do not have
> support for this case in `AssembleMove` so this patch removes it. Furthermore,
> if we add a test for this case, we see that a `push ip` instruction is
> generated, effectively pushing whatever was in `ip` at the time instead of
> pushing a constant.
>
> Concerning stack slot pushes, we generate a more or less equivalent sequence of
> instructions.
>
> Finally, grouping floating point pushes is not used anywhere so this patch
> removes support for this also.
>
> Bug: v8:6553
> Change-Id: I9b820d33361fc442dd813f66e1f96cda41009110
> Reviewed-on: https://chromium-review.googlesource.com/567191
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
> Cr-Commit-Position: refs/heads/master@{#46718}
Bug: v8:6553
Change-Id: Ib9a55dae7cc5db6185d163c56088ff23426d04bb
Reviewed-on: https://chromium-review.googlesource.com/576087
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Cr-Commit-Position: refs/heads/master@{#46754}
This reverts commit a72b2f88a8.
Reason for revert: Breaks https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20debug/builds/7093
Original change's description:
> [arm] Restrict grouping pushes before a TailCall to registers only
>
> We optimize parallel moves performed before a TailCall by grouping adjacent
> pushes. This way, we may use a single instruction to push multiple registers at
> once. However, we also have support for pushing immediates and stack slots for
> which the benefit is questionnable therefore this patch removes support for
> them.
>
> Concerning immediate pushes, it looks like a mistake since we do not have
> support for this case in `AssembleMove` so this patch removes it. Furthermore,
> if we add a test for this case, we see that a `push ip` instruction is
> generated, effectively pushing whatever was in `ip` at the time instead of
> pushing a constant.
>
> Concerning stack slot pushes, we generate a more or less equivalent sequence of
> instructions.
>
> Finally, grouping floating point pushes is not used anywhere so this patch
> removes support for this also.
>
> Bug: v8:6553
> Change-Id: I9b820d33361fc442dd813f66e1f96cda41009110
> Reviewed-on: https://chromium-review.googlesource.com/567191
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
> Cr-Commit-Position: refs/heads/master@{#46718}
TBR=danno@chromium.org,jarin@chromium.org,pierre.langlois@arm.com,bmeurer@chromium.org
Change-Id: Ib9db9e6e4f033aeea32741e04b1b884429acc800
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6553
Reviewed-on: https://chromium-review.googlesource.com/574908
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46719}
We optimize parallel moves performed before a TailCall by grouping adjacent
pushes. This way, we may use a single instruction to push multiple registers at
once. However, we also have support for pushing immediates and stack slots for
which the benefit is questionnable therefore this patch removes support for
them.
Concerning immediate pushes, it looks like a mistake since we do not have
support for this case in `AssembleMove` so this patch removes it. Furthermore,
if we add a test for this case, we see that a `push ip` instruction is
generated, effectively pushing whatever was in `ip` at the time instead of
pushing a constant.
Concerning stack slot pushes, we generate a more or less equivalent sequence of
instructions.
Finally, grouping floating point pushes is not used anywhere so this patch
removes support for this also.
Bug: v8:6553
Change-Id: I9b820d33361fc442dd813f66e1f96cda41009110
Reviewed-on: https://chromium-review.googlesource.com/567191
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Cr-Commit-Position: refs/heads/master@{#46718}
