Commit 85990489 authored by Ng Zhi An's avatar Ng Zhi An Committed by Commit Bot

[wasm-simd][liftoff] Fix load from stack into reg for PrepareCall

Take care of the case when loading from stack slot into registers during
PrepareCall. register_loads_ knows nothing of fp pair (it calls
liftoff_code, which is nonsense for an fp pair), so we pass it dst.low().

Earlier on in LoadStackSlot, we call load_dst_regs_.set(dst). This is
implement to set both dst.low() and dst.high(). Later when we iterate
over load_dst_regs_ (in ExecuteLoads), we special case loads of
kWasmS128, and reconstruct a LiftoffRegister that is a fp pair (only
when required), since LiftoffAssembler::Fill knows how to handle loading
of s128 values. However, since we also set dst.high(), we will iterate
over it too, but end up getting a RegisterLoad of nonsense values. This
is not handled in the switch cases at all, so it works.

Instead of this, we could transform a s128 load into two f64 load, and
PrepareCall will be the only place that does this, since other callers
of Fill will use kWasmS128. And we would have to do 2 loads instead of
1 add and 1 load.

Bug: v8:9909
Change-Id: I20f69de48a650b457a199fde02c8e58641b2c176
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2013920Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65927}
parent 5005a841
...@@ -212,6 +212,9 @@ class StackTransferRecipe { ...@@ -212,6 +212,9 @@ class StackTransferRecipe {
RegisterLoad::HalfStack(stack_offset, kLowWord); RegisterLoad::HalfStack(stack_offset, kLowWord);
*register_load(dst.high()) = *register_load(dst.high()) =
RegisterLoad::HalfStack(stack_offset, kHighWord); RegisterLoad::HalfStack(stack_offset, kHighWord);
} else if (dst.is_fp_pair()) {
DCHECK_EQ(kWasmS128, type);
*register_load(dst.low()) = RegisterLoad::Stack(stack_offset, type);
} else { } else {
*register_load(dst) = RegisterLoad::Stack(stack_offset, type); *register_load(dst) = RegisterLoad::Stack(stack_offset, type);
} }
...@@ -314,7 +317,12 @@ class StackTransferRecipe { ...@@ -314,7 +317,12 @@ class StackTransferRecipe {
: WasmValue(int32_t{load->value})); : WasmValue(int32_t{load->value}));
break; break;
case RegisterLoad::kStack: case RegisterLoad::kStack:
asm_->Fill(dst, load->value, load->type); if (kNeedS128RegPair && load->type == kWasmS128) {
asm_->Fill(LiftoffRegister::ForFpPair(dst.fp()), load->value,
load->type);
} else {
asm_->Fill(dst, load->value, load->type);
}
break; break;
case RegisterLoad::kLowHalfStack: case RegisterLoad::kLowHalfStack:
// Half of a register pair, {dst} must be a gp register. // Half of a register pair, {dst} must be a gp register.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment