Skip to content

Commit

Permalink
[maglev][arm] Fix many small issues
Browse files Browse the repository at this point in the history 
- ToUint8Clamped depends on ARMv8 (It is only emitted if
Float64Round is supported.
- Comparison on div.
- Spurious DebugBreak on mod.
- Shifting (immediate) 0 on Arm shifts 32 instead.
- Fixed register for CallCFunction inputs
- Similarly to Arm64, on Arm Float64Modules is a call to a builtin.
- TestTypeOf needs an extra register on Arm.
- Fixes PushAll order
- Fixes OSR bit tests

Drive-by: Remove mjsunit exceptions for Arm.

Bug: v8:7700
Change-Id: I20f468469aec0b116c3dd750e3630479e25f3320
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4651873
Reviewed-by: 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@{#88550}
  • Loading branch information
@victorgomes
victorgomes authored and V8 LUCI CQ committed Jun 29, 2023
1 parent 5266b56 commit bb6da6f
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/codegen/arm/macro-assembler-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2975,14 +2975,14 @@ void MacroAssembler::Switch(Register scratch, Register value,
void MacroAssembler::JumpIfCodeIsMarkedForDeoptimization(
Register code, Register scratch, Label* if_marked_for_deoptimization) {
ldr(scratch, FieldMemOperand(code, Code::kFlagsOffset));
tst(scratch, Operand(Code::kMarkedForDeoptimizationBit));
tst(scratch, Operand(1 << Code::kMarkedForDeoptimizationBit));
b(if_marked_for_deoptimization, ne);
}

void MacroAssembler::JumpIfCodeIsTurbofanned(Register code, Register scratch,
Label* if_turbofanned) {
ldr(scratch, FieldMemOperand(code, Code::kFlagsOffset));
tst(scratch, Operand(Code::kIsTurbofannedBit));
tst(scratch, Operand(1 << Code::kIsTurbofannedBit));
b(if_turbofanned, ne);
}

Expand Down
13 changes: 11 additions & 2 deletions src/codegen/arm/macro-assembler-arm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,13 +561,22 @@ class V8_EXPORT_PRIVATE MacroAssembler : public MacroAssemblerBase {
void PushAll(RegList registers) {
if (registers.is_empty()) return;
ASM_CODE_COMMENT(this);
stm(db_w, sp, registers);
// stm(db_w, sp, registers);
// TODO(victorgomes): {stm/ldm} pushes/pops registers in the opposite order
// as expected by Maglev frame. Consider massaging Maglev to accept this
// order instead.
for (Register reg : registers) {
push(reg);
}
}

void PopAll(RegList registers) {
if (registers.is_empty()) return;
ASM_CODE_COMMENT(this);
ldm(ia_w, sp, registers);
// ldm(ia_w, sp, registers);
for (Register reg : base::Reversed(registers)) {
pop(reg);
}
}

void PushAll(DoubleRegList registers, int stack_slot_size = kDoubleSize) {
Expand Down
1 change: 1 addition & 0 deletions src/maglev/arm/maglev-assembler-arm-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ inline void MaglevAssembler::NegateInt32(Register val) {
inline void MaglevAssembler::ToUint8Clamped(Register result,
DoubleRegister value, Label* min,
Label* max, Label* done) {
CpuFeatureScope scope(this, ARMv8);
ScratchRegisterScope temps(this);
DoubleRegister scratch = temps.AcquireDouble();
Move(scratch, 0.0);
Expand Down
73 changes: 40 additions & 33 deletions src/maglev/arm/maglev-ir-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ void Int32DivideWithOverflow::GenerateCode(MaglevAssembler* masm,
// Check that the remainder is zero.
Register temp = temps.Acquire();
__ mul(temp, res, right);
__ tst(temp, left);
__ cmp(temp, left);
__ EmitEagerDeoptIf(ne, DeoptimizeReason::kNotInt32, this);

__ Move(out, res);
Expand Down Expand Up @@ -435,7 +435,6 @@ void Int32ModulusWithOverflow::GenerateCode(MaglevAssembler* masm,
__ add(mask, rhs, Operand(-1));
__ tst(mask, rhs);
__ JumpIf(ne, &rhs_not_power_of_2);
__ DebugBreak();

// {rhs} is power of 2.
__ and_(out, mask, lhs);
Expand Down Expand Up @@ -467,31 +466,38 @@ DEF_BITWISE_BINOP(Int32BitwiseOr, orr)
DEF_BITWISE_BINOP(Int32BitwiseXor, eor)
#undef DEF_BITWISE_BINOP

#define DEF_SHIFT_BINOP(Instruction, opcode) \
void Instruction::SetValueLocationConstraints() { \
UseRegister(left_input()); \
if (right_input().node()->Is<Int32Constant>()) { \
UseAny(right_input()); \
} else { \
UseRegister(right_input()); \
} \
DefineAsRegister(this); \
} \
void Instruction::GenerateCode(MaglevAssembler* masm, \
const ProcessingState& state) { \
Register left = ToRegister(left_input()); \
Register out = ToRegister(result()); \
if (Int32Constant* constant = \
right_input().node()->TryCast<Int32Constant>()) { \
__ opcode(out, left, \
Operand(static_cast<uint32_t>(constant->value()) & 31)); \
} else { \
MaglevAssembler::ScratchRegisterScope temps(masm); \
Register scratch = temps.Acquire(); \
Register right = ToRegister(right_input()); \
__ and_(scratch, right, Operand(31)); \
__ opcode(out, left, Operand(scratch)); \
} \
#define DEF_SHIFT_BINOP(Instruction, opcode) \
void Instruction::SetValueLocationConstraints() { \
UseRegister(left_input()); \
if (right_input().node()->Is<Int32Constant>()) { \
UseAny(right_input()); \
} else { \
UseRegister(right_input()); \
} \
DefineAsRegister(this); \
} \
void Instruction::GenerateCode(MaglevAssembler* masm, \
const ProcessingState& state) { \
Register left = ToRegister(left_input()); \
Register out = ToRegister(result()); \
if (Int32Constant* constant = \
right_input().node()->TryCast<Int32Constant>()) { \
if (constant->value() == 0) { \
/* TODO(victorgomes): Arm will do a shift of 32 if right == 0. Ideally \
* we should not even emit the shift in the first place. We do a move \
* here for the moment. */ \
__ Move(out, left); \
return; \
} \
__ opcode(out, left, \
Operand(static_cast<uint32_t>(constant->value()) & 31)); \
} else { \
MaglevAssembler::ScratchRegisterScope temps(masm); \
Register scratch = temps.Acquire(); \
Register right = ToRegister(right_input()); \
__ and_(scratch, right, Operand(31)); \
__ opcode(out, left, Operand(scratch)); \
} \
}
DEF_SHIFT_BINOP(Int32ShiftLeft, lsl)
DEF_SHIFT_BINOP(Int32ShiftRight, asr)
Expand Down Expand Up @@ -566,9 +572,10 @@ void Float64Divide::GenerateCode(MaglevAssembler* masm,
__ vdiv(out, left, right);
}

int Float64Modulus::MaxCallStackArgs() const { return 0; }
void Float64Modulus::SetValueLocationConstraints() {
UseRegister(left_input());
UseRegister(right_input());
UseFixed(left_input(), d0);
UseFixed(right_input(), d1);
DefineAsRegister(this);
}
void Float64Modulus::GenerateCode(MaglevAssembler* masm,
Expand Down Expand Up @@ -625,8 +632,8 @@ void Float64Round::GenerateCode(MaglevAssembler* masm,

int Float64Exponentiate::MaxCallStackArgs() const { return 0; }
void Float64Exponentiate::SetValueLocationConstraints() {
UseRegister(left_input());
UseRegister(right_input());
UseFixed(left_input(), d0);
UseFixed(right_input(), d1);
DefineAsRegister(this);
}
void Float64Exponentiate::GenerateCode(MaglevAssembler* masm,
Expand All @@ -643,7 +650,7 @@ void Float64Exponentiate::GenerateCode(MaglevAssembler* masm,

int Float64Ieee754Unary::MaxCallStackArgs() const { return 0; }
void Float64Ieee754Unary::SetValueLocationConstraints() {
UseRegister(input());
UseFixed(input(), d0);
DefineAsRegister(this);
}
void Float64Ieee754Unary::GenerateCode(MaglevAssembler* masm,
Expand Down Expand Up @@ -804,7 +811,7 @@ void GenerateReduceInterruptBudget(MaglevAssembler* masm, Node* node,
FieldMemOperand(feedback_cell, JSFunction::kFeedbackCellOffset));
__ ldr(budget,
FieldMemOperand(feedback_cell, FeedbackCell::kInterruptBudgetOffset));
__ sub(budget, budget, Operand(amount));
__ sub(budget, budget, Operand(amount), SetCC);
__ str(budget,
FieldMemOperand(feedback_cell, FeedbackCell::kInterruptBudgetOffset));
ZoneLabelRef done(masm);
Expand Down
3 changes: 3 additions & 0 deletions src/maglev/maglev-ir.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3929,6 +3929,9 @@ void TestInstanceOf::GenerateCode(MaglevAssembler* masm,
void TestTypeOf::SetValueLocationConstraints() {
UseRegister(value());
DefineAsRegister(this);
#ifdef V8_TARGET_ARCH_ARM
set_temporaries_needed(1);
#endif
}
void TestTypeOf::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Expand Down
4 changes: 2 additions & 2 deletions src/maglev/maglev-ir.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2651,8 +2651,8 @@ DEF_FLOAT64_BINARY_NODE(Add)
DEF_FLOAT64_BINARY_NODE(Subtract)
DEF_FLOAT64_BINARY_NODE(Multiply)
DEF_FLOAT64_BINARY_NODE(Divide)
#ifdef V8_TARGET_ARCH_ARM64
// On Arm64, floating point modulus is implemented with a call to a C++
#if defined(V8_TARGET_ARCH_ARM64) || defined(V8_TARGET_ARCH_ARM)
// On Arm/Arm64, floating point modulus is implemented with a call to a C++
// function, while on x64, it's implemented natively without call.
DEF_FLOAT64_BINARY_NODE_WITH_CALL(Modulus)
#else
Expand Down

0 comments on commit bb6da6f

Please sign in to comment.