Skip to content

Commit

Permalink
Revert "[maglev] Specialize CheckedInternalizedString and CheckNumber…
Browse files Browse the repository at this point in the history
… for x64"

This reverts commit 7f5755f.

Reason for revert: That didn´t fix the regression in the bots.

Original change's description:
> [maglev] Specialize CheckedInternalizedString and CheckNumber for x64
>
> x64 is able to compare a map to a root (or an instance type to a mask)
> without loading to a register first.
>
> This is a partial revert of https://crrev.com/c/4650731
>
> Bug: chromium:1461980
> Change-Id: I49c279827011c51f054184e3a17eaa2009b1fdee
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4661662
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Auto-Submit: Victor Gomes <victorgomes@chromium.org>
> Commit-Queue: Victor Gomes <victorgomes@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#88649}

Bug: chromium:1461980
Change-Id: Ib2bf8ff21ef45a7872c0305b9b78a107bcc1dc32
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4666724
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#88664}
  • Loading branch information
victorgomes authored and V8 LUCI CQ committed Jul 5, 2023
1 parent 8535fe2 commit d5456d3
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 90 deletions.
13 changes: 0 additions & 13 deletions src/maglev/arm/maglev-assembler-arm-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -667,19 +667,6 @@ inline Condition MaglevAssembler::IsNotCallableNorUndetactable(
return kEqual;
}

inline void MaglevAssembler::CheckHeapObjectIsNumeric(Register heap_object,
Label* fail) {
ScratchRegisterScope temps(this);
Register scratch = temps.Acquire();
Label done;
LoadMap(scratch, heap_object);
CompareRoot(scratch, RootIndex::kHeapNumberMap);
JumpIf(kEqual, &done, Label::Distance::kNear);
CompareRoot(scratch, RootIndex::kBigIntMap);
JumpIf(kNotEqual, fail);
bind(&done);
}

inline void MaglevAssembler::LoadInstanceType(Register instance_type,
Register heap_object) {
LoadMap(instance_type, heap_object);
Expand Down
13 changes: 0 additions & 13 deletions src/maglev/arm64/maglev-assembler-arm64-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -739,19 +739,6 @@ inline Condition MaglevAssembler::IsNotCallableNorUndetactable(
return kEqual;
}

inline void MaglevAssembler::CheckHeapObjectIsNumeric(Register heap_object,
Label* fail) {
ScratchRegisterScope temps(this);
Register scratch = temps.Acquire();
Label done;
LoadMap(scratch, heap_object);
CompareRoot(scratch, RootIndex::kHeapNumberMap);
JumpIf(kEqual, &done, Label::Distance::kNear);
CompareRoot(scratch, RootIndex::kBigIntMap);
JumpIf(kNotEqual, fail);
bind(&done);
}

inline void MaglevAssembler::LoadInstanceType(Register instance_type,
Register heap_object) {
LoadMap(instance_type, heap_object);
Expand Down
2 changes: 0 additions & 2 deletions src/maglev/maglev-assembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,6 @@ class MaglevAssembler : public MacroAssembler {
inline Condition IsCallableAndNotUndetectable(Register map, Register scratch);
inline Condition IsNotCallableNorUndetactable(Register map, Register scratch);

inline void CheckHeapObjectIsNumeric(Register heap_object, Label* fail);

inline void LoadInstanceType(Register instance_type, Register heap_object);
inline void IsObjectType(Register heap_object, InstanceType type);
inline void CompareObjectType(Register heap_object, InstanceType type);
Expand Down
21 changes: 12 additions & 9 deletions src/maglev/maglev-ir.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4392,26 +4392,30 @@ void CheckNumber::SetValueLocationConstraints() {
}
void CheckNumber::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register value = ToRegister(receiver_input());
Label* deopt = __ GetDeoptLabel(this, DeoptimizeReason::kNotANumber);
Label done;
MaglevAssembler::ScratchRegisterScope temps(masm);
Register scratch = temps.GetDefaultScratchRegister();
Register value = ToRegister(receiver_input());
// If {value} is a Smi or a HeapNumber, we're done.
__ JumpIfSmi(value, &done, Label::Distance::kNear);
if (mode() == Object::Conversion::kToNumeric) {
__ CheckHeapObjectIsNumeric(value, deopt);
__ LoadMap(scratch, value);
__ CompareRoot(scratch, RootIndex::kHeapNumberMap);
// Jump to done if it is a HeapNumber.
__ JumpIf(kEqual, &done, Label::Distance::kNear);
// Check if it is a BigInt.
__ CompareRoot(scratch, RootIndex::kBigIntMap);
} else {
__ JumpIfNotRoot(value, RootIndex::kHeapNumberMap, deopt);
__ CompareMapWithRoot(value, RootIndex::kHeapNumberMap, scratch);
}
__ EmitEagerDeoptIf(kNotEqual, DeoptimizeReason::kNotANumber, this);
__ bind(&done);
}

void CheckedInternalizedString::SetValueLocationConstraints() {
UseRegister(object_input());
DefineSameAsFirst(this);
}

#ifndef V8_TARGET_ARCH_X64
// We specialize CheckedInternalizedString for x64. Since x64 can compare a map
// to a root (or an instance type to a mask) without loading to a register.
void CheckedInternalizedString::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(object_input());
Expand Down Expand Up @@ -4456,7 +4460,6 @@ void CheckedInternalizedString::GenerateCode(MaglevAssembler* masm,
done, this, object, instance_type));
__ bind(*done);
}
#endif // V8_TARGET_ARCH_X64

void CheckedNumberToUint8Clamped::SetValueLocationConstraints() {
UseRegister(input());
Expand Down
10 changes: 0 additions & 10 deletions src/maglev/x64/maglev-assembler-x64-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -629,16 +629,6 @@ inline Condition MaglevAssembler::IsNotCallableNorUndetactable(
return kEqual;
}

inline void MaglevAssembler::CheckHeapObjectIsNumeric(Register heap_object,
Label* fail) {
Label done;
CompareMapWithRoot(heap_object, RootIndex::kHeapNumberMap, kScratchRegister);
JumpIf(kEqual, &done, Label::Distance::kNear);
CompareMapWithRoot(heap_object, RootIndex::kBigIntMap, kScratchRegister);
JumpIf(kNotEqual, fail);
bind(&done);
}

inline void MaglevAssembler::LoadInstanceType(Register instance_type,
Register heap_object) {
LoadMap(instance_type, heap_object);
Expand Down
43 changes: 0 additions & 43 deletions src/maglev/x64/maglev-ir-x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,49 +100,6 @@ int CheckedObjectToIndex::MaxCallStackArgs() const {
return MaglevAssembler::ArgumentStackSlotsForCFunctionCall(1);
}

void CheckedInternalizedString::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(object_input());

if (check_type() == CheckType::kOmitHeapObjectCheck) {
__ AssertNotSmi(object);
} else {
Condition is_smi = __ CheckSmi(object);
__ EmitEagerDeoptIf(is_smi, DeoptimizeReason::kWrongMap, this);
}

__ LoadMap(kScratchRegister, object);
__ RecordComment("Test IsInternalizedString");
// Go to the slow path if this is a non-string, or a non-internalised string.
__ testw(FieldOperand(kScratchRegister, Map::kInstanceTypeOffset),
Immediate(kIsNotStringMask | kIsNotInternalizedMask));
static_assert((kStringTag | kInternalizedTag) == 0);
ZoneLabelRef done(masm);
__ JumpToDeferredIf(
not_zero,
[](MaglevAssembler* masm, ZoneLabelRef done, Register object,
CheckedInternalizedString* node, EagerDeoptInfo* deopt_info) {
__ RecordComment("Deferred Test IsThinString");
__ cmpw(FieldOperand(kScratchRegister, Map::kInstanceTypeOffset),
Immediate(THIN_STRING_TYPE));
// Deopt if this isn't a thin string.
__ EmitEagerDeoptIf(not_equal, DeoptimizeReason::kWrongMap, node);
__ LoadTaggedField(object,
FieldOperand(object, ThinString::kActualOffset));
if (v8_flags.debug_code) {
__ RecordComment("DCHECK IsInternalizedString");
__ LoadMap(kScratchRegister, object);
__ testw(FieldOperand(kScratchRegister, Map::kInstanceTypeOffset),
Immediate(kIsNotStringMask | kIsNotInternalizedMask));
static_assert((kStringTag | kInternalizedTag) == 0);
__ Check(zero, AbortReason::kUnexpectedValue);
}
__ jmp(*done);
},
done, object, this, eager_deopt_info());
__ bind(*done);
}

int BuiltinStringFromCharCode::MaxCallStackArgs() const {
return AllocateDescriptor::GetStackParameterCount();
}
Expand Down

0 comments on commit d5456d3

Please sign in to comment.