AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm][liftoff] Relax stack slot compatibility requirements

Browse Source 
Since we don't do accurate type tracking in liftoff, we end up in
situation where we mix up ref and (ref null). This is safe and should
be allowed.
We merge {IsAssignable} into {CheckCompatibleStackSlotTypes}, and
rename and simplify it.

Bug: v8:13499
Change-Id: Ifaa2ff1e3f090a5d91219305ce4bb6f08bc5c00f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4030512
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84330}
This commit is contained in:
Manos Koukoutos 2022-11-17 11:48:15 +01:00 committed by V8 LUCI CQ
parent 0446de4202
commit 3f3e218057
3 changed files with 11 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/wasm/baseline/liftoff-assembler.cc
View File

@ -93,7 +93,7 @@ class StackTransferRecipe {
}
V8_INLINE void TransferStackSlot(const VarState& dst, const VarState& src) {
DCHECK(CheckCompatibleStackSlotTypes(dst.kind(), src.kind()));
DCHECK(CompatibleStackSlotTypes(dst.kind(), src.kind()));
if (dst.is_reg()) {
LoadIntoRegister(dst.reg(), src, src.offset());
return;
@ -978,7 +978,7 @@ void PrepareStackTransfers(const ValueKindSig* sig,
const int num_lowered_params = is_gp_pair ? 2 : 1;
const VarState& slot = slots[param];
const uint32_t stack_offset = slot.offset();
DCHECK(IsAssignable(slot.kind(), kind));
DCHECK(CompatibleStackSlotTypes(slot.kind(), kind));
// Process both halfs of a register pair separately, because they are passed
// as separate parameters. One or both of them could end up on the stack.
for (int lowered_idx = 0; lowered_idx < num_lowered_params; ++lowered_idx) {
@ -1426,27 +1426,11 @@ std::ostream& operator<<(std::ostream& os, VarState slot) {
}
#if DEBUG
bool CheckCompatibleStackSlotTypes(ValueKind a, ValueKind b) {
if (is_object_reference(a)) {
// Since Liftoff doesn't do accurate type tracking (e.g. on loop back
// edges), we only care that pointer types stay amongst pointer types.
// It's fine if ref/ref null overwrite each other.
DCHECK(is_object_reference(b));
} else if (is_rtt(a)) {
// Same for rtt/rtt_with_depth.
DCHECK(is_rtt(b));
} else {
// All other types (primitive numbers, bottom/stmt) must be equal.
DCHECK_EQ(a, b);
}
return true; // Dummy so this can be called via DCHECK.
}
bool IsAssignable(ValueKind src, ValueKind dst) {
if (src == dst) return true;
if (src == kRef && dst == kRefNull) return true;
if (src == kRtt && (dst == kRef || dst == kRefNull)) return true;
return false;
bool CompatibleStackSlotTypes(ValueKind a, ValueKind b) {
// Since Liftoff doesn't do accurate type tracking (e.g. on loop back edges,
// ref.as_non_null/br_on_cast results), we only care that pointer types stay
// amongst pointer types. It's fine if ref/ref null overwrite each other.
return a == b || (is_object_reference(a) && is_object_reference(b));
}
#endif

3
src/wasm/baseline/liftoff-assembler.h
View File

@ -1850,8 +1850,7 @@ class LiftoffStackSlots {
};
#if DEBUG
bool CheckCompatibleStackSlotTypes(ValueKind a, ValueKind b);
bool IsAssignable(ValueKind src, ValueKind dst);
bool CompatibleStackSlotTypes(ValueKind a, ValueKind b);
#endif
} // namespace wasm

6
src/wasm/baseline/liftoff-compiler.cc
View File

@ -2394,7 +2394,7 @@ class LiftoffCompiler {
state.dec_used(slot_reg);
dst_slot->MakeStack();
}
DCHECK(CheckCompatibleStackSlotTypes(kind, __ local_kind(local_index)));
DCHECK(CompatibleStackSlotTypes(kind, __ local_kind(local_index)));
RegClass rc = reg_class_for(kind);
LiftoffRegister dst_reg = __ GetUnusedRegister(rc, {});
__ Fill(dst_reg, src_slot.offset(), kind);
@ -2673,7 +2673,7 @@ class LiftoffCompiler {
LiftoffRegList pinned;
Register condition = pinned.set(__ PopToRegister()).gp();
ValueKind kind = __ cache_state()->stack_state.end()[-1].kind();
DCHECK(CheckCompatibleStackSlotTypes(
DCHECK(CompatibleStackSlotTypes(
kind, __ cache_state()->stack_state.end()[-2].kind()));
LiftoffRegister false_value = pinned.set(__ PopToRegister(pinned));
LiftoffRegister true_value = __ PopToRegister(pinned);
@ -3467,7 +3467,7 @@ class LiftoffCompiler {
? decoder->local_type(index)
: exception ? ValueType::Ref(HeapType::kAny)
: decoder->stack_value(decoder_stack_index--)->type;
DCHECK(CheckCompatibleStackSlotTypes(slot.kind(), type.kind()));
DCHECK(CompatibleStackSlotTypes(slot.kind(), type.kind()));
value.type = type;
switch (slot.loc()) {
case kIntConst: