[maglev] Avoid copying temporary registers to deferred code

... where scratch scope is no longer active and the registers
could be clobbered.

Bug: v8:7700
Change-Id: I366312e2cafd732199b4668fd5f40363688c8a04
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4107089
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84897}
This commit is contained in:
Victor Gomes 2022-12-16 10:58:00 +01:00 committed by V8 LUCI CQ
parent 4c4edc85ea
commit 94f80d253c
4 changed files with 62 additions and 40 deletions

View File

@ -45,6 +45,9 @@ namespace internal {
ALWAYS_ALLOCATABLE_GENERAL_REGISTERS(V) \
MAYBE_ALLOCATABLE_GENERAL_REGISTERS(V)
#define MAGLEV_SCRATCH_GENERAL_REGISTERS(R) \
R(x16) R(x17)
#define FLOAT_REGISTERS(V) \
V(s0) V(s1) V(s2) V(s3) V(s4) V(s5) V(s6) V(s7) \
V(s8) V(s9) V(s10) V(s11) V(s12) V(s13) V(s14) V(s15) \
@ -76,6 +79,10 @@ namespace internal {
R(d8) R(d9) R(d10) R(d11) R(d12) R(d13) R(d14) R(d16) \
R(d17) R(d18) R(d19) R(d20) R(d21) R(d22) R(d23) R(d24) \
R(d25) R(d26) R(d27) R(d28)
#define MAGLEV_SCRATCH_DOUBLE_REGISTERS(R) \
R(d30) R(d31)
// clang-format on
// Some CPURegister methods can return Register and VRegister types, so we

View File

@ -199,14 +199,16 @@ void MaglevAssembler::Prologue(Graph* graph) {
AssertFeedbackVector(feedback_vector, flags);
DeferredCodeInfo* deferred_flags_need_processing = PushDeferredCode(
[](MaglevAssembler* masm, Register flags, Register feedback_vector) {
[](MaglevAssembler* masm, Register feedback_vector) {
ASM_CODE_COMMENT_STRING(masm, "Optimized marker check");
// TODO(leszeks): This could definitely be a builtin that we
// tail-call.
UseScratchRegisterScope temps(masm);
Register flags = temps.AcquireX();
__ OptimizeCodeOrTailCallOptimizedCodeSlot(flags, feedback_vector);
__ Trap();
},
flags, feedback_vector);
feedback_vector);
LoadFeedbackVectorFlagsAndJumpIfNeedsProcessing(
flags, feedback_vector, CodeKind::MAGLEV,

View File

@ -497,10 +497,9 @@ void Int32DivideWithOverflow::GenerateCode(MaglevAssembler* masm,
}
void Int32ModulusWithOverflow::SetValueLocationConstraints() {
UseRegister(left_input());
UseRegister(right_input());
UseAndClobberRegister(left_input());
UseAndClobberRegister(right_input());
DefineAsRegister(this);
set_temporaries_needed(1);
}
void Int32ModulusWithOverflow::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
@ -521,16 +520,11 @@ void Int32ModulusWithOverflow::GenerateCode(MaglevAssembler* masm,
// lhs % rhs
Register left = ToRegister(left_input()).W();
Register right_maybe_neg = ToRegister(right_input()).W();
Register right = ToRegister(right_input()).W();
Register out = ToRegister(result()).W();
ZoneLabelRef done(masm);
ZoneLabelRef rhs_checked(masm);
UseScratchRegisterScope temps(masm);
Register right = temps.AcquireW();
__ Move(right, right_maybe_neg);
__ Cmp(right, Immediate(0));
__ JumpToDeferredIf(
le,
@ -544,30 +538,26 @@ void Int32ModulusWithOverflow::GenerateCode(MaglevAssembler* masm,
__ bind(*rhs_checked);
__ Cmp(left, Immediate(0));
{
UseScratchRegisterScope temps(masm);
Register scratch = temps.AcquireW();
__ JumpToDeferredIf(
lt,
[](MaglevAssembler* masm, ZoneLabelRef done, Register left_neg,
Register right, Register out, Register scratch,
Int32ModulusWithOverflow* node) {
Register left = scratch;
Register res = node->general_temporaries().first().W();
__ neg(left, left_neg);
__ udiv(res, left, right);
__ msub(out, res, right, left);
__ cmp(out, Immediate(0));
// TODO(victorgomes): This ideally should be kMinusZero, but Maglev
// only allows one deopt reason per IR.
__ EmitEagerDeoptIf(eq, DeoptimizeReason::kDivisionByZero, node);
__ neg(out, out);
__ b(*done);
},
done, left, right, out, scratch, this);
}
__ JumpToDeferredIf(
lt,
[](MaglevAssembler* masm, ZoneLabelRef done, Register left,
Register right, Register out, Int32ModulusWithOverflow* node) {
UseScratchRegisterScope temps(masm);
Register res = temps.AcquireW();
__ neg(left, left);
__ udiv(res, left, right);
__ msub(out, res, right, left);
__ cmp(out, Immediate(0));
// TODO(victorgomes): This ideally should be kMinusZero, but Maglev
// only allows one deopt reason per IR.
__ EmitEagerDeoptIf(eq, DeoptimizeReason::kDivisionByZero, node);
__ neg(out, out);
__ b(*done);
},
done, left, right, out, this);
Label right_not_power_of_2;
UseScratchRegisterScope temps(masm);
Register mask = temps.AcquireW();
__ Add(mask, right, Immediate(-1));
__ Tst(mask, right);
@ -817,6 +807,7 @@ void CheckInt32IsSmi::GenerateCode(MaglevAssembler* masm,
void CheckedSmiTagInt32::SetValueLocationConstraints() {
UseRegister(input());
DefineAsRegister(this);
set_temporaries_needed(1);
}
void CheckedSmiTagInt32::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
@ -833,11 +824,11 @@ void CheckedSmiTagInt32::GenerateCode(MaglevAssembler* masm,
void CheckedInternalizedString::SetValueLocationConstraints() {
UseRegister(object_input());
DefineSameAsFirst(this);
set_temporaries_needed(1);
}
void CheckedInternalizedString::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
UseScratchRegisterScope temps(masm);
Register scratch = temps.AcquireX();
Register scratch = general_temporaries().PopFirst();
Register object = ToRegister(object_input());
if (check_type_ == CheckType::kOmitHeapObjectCheck) {
@ -850,7 +841,7 @@ void CheckedInternalizedString::GenerateCode(MaglevAssembler* masm,
__ LoadMap(scratch, object);
__ RecordComment("Test IsInternalizedString");
// Go to the slow path if this is a non-string, or a non-internalised string.
__ Ldrh(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset));
__ Ldr(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset));
__ Tst(scratch, Immediate(kIsNotStringMask | kIsNotInternalizedMask));
static_assert((kStringTag | kInternalizedTag) == 0);
ZoneLabelRef done(masm);

View File

@ -271,6 +271,15 @@ struct CopyForDeferredByValue {
}
};
#ifdef V8_TARGET_ARCH_ARM64
#define LIST_REG(V) V,
static constexpr RegList kScratchGeneralRegisters = {
MAGLEV_SCRATCH_GENERAL_REGISTERS(LIST_REG) Register::no_reg()};
static constexpr DoubleRegList kScratchDoubleRegisters = {
MAGLEV_SCRATCH_DOUBLE_REGISTERS(LIST_REG) DoubleRegister::no_reg()};
#undef LIST_REG
#endif // V8_TARGET_ARCH_ARM64
// Node pointers are copied by value.
template <typename T>
struct CopyForDeferredHelper<
@ -291,11 +300,24 @@ struct CopyForDeferredHelper<MaglevCompilationInfo*>
: public CopyForDeferredByValue<MaglevCompilationInfo*> {};
// Machine registers are copied by value.
template <>
struct CopyForDeferredHelper<Register>
: public CopyForDeferredByValue<Register> {};
struct CopyForDeferredHelper<Register> {
static Register Copy(MaglevCompilationInfo* compilation_info, Register reg) {
#ifdef V8_TARGET_ARCH_ARM64
DCHECK(!kScratchGeneralRegisters.has(reg));
#endif // V8_TARGET_ARCH_ARM64
return reg;
}
};
template <>
struct CopyForDeferredHelper<DoubleRegister>
: public CopyForDeferredByValue<DoubleRegister> {};
struct CopyForDeferredHelper<DoubleRegister> {
static DoubleRegister Copy(MaglevCompilationInfo* compilation_info,
DoubleRegister reg) {
#ifdef V8_TARGET_ARCH_ARM64
DCHECK(!kScratchDoubleRegisters.has(reg));
#endif // V8_TARGET_ARCH_ARM64
return reg;
}
};
// Bytecode offsets are copied by value.
template <>
struct CopyForDeferredHelper<BytecodeOffset>