From 82c7f93aebbaaca3eb6da70df499dcd29497295a Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Tue, 10 Jan 2023 10:25:53 +0000 Subject: [PATCH] Revert "[maglev] Force (U)Int32 values to always be zero extended" This reverts commit fe54336953e1d6a9ab6a3897880f1cdf78d4d3fa. Reason for revert: Android build failure: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Android%20Arm64%20-%20debug%20builder/28804/overview Original change's description: > [maglev] Force (U)Int32 values to always be zero extended > > Ensure that (U)Int32 values are always zero extended (in particular, > after Float64 truncation and constant materialisation), and add debug > code which asserts that (U)Int32 register inputs to nodes are zero > extended at input read time. > > Bug: v8:7700 > Change-Id: Idbebabdd48bc7a6d2d73f1dfce7da629b5814ca5 > Fixed: chromium:1404066 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4147621 > Reviewed-by: Victor Gomes > Auto-Submit: Leszek Swirski > Commit-Queue: Leszek Swirski > Cr-Commit-Position: refs/heads/main@{#85169} Bug: v8:7700 Change-Id: Ifa7a5541fda93522d1ea34dafdfc88d2561a8795 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4151488 Auto-Submit: Leszek Swirski Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#85170} --- src/codegen/arm64/macro-assembler-arm64.cc | 7 ------- src/codegen/arm64/macro-assembler-arm64.h | 4 ---- src/maglev/arm64/maglev-assembler-arm64-inl.h | 2 +- src/maglev/arm64/maglev-assembler-arm64.cc | 3 --- src/maglev/maglev-assembler.h | 2 -- src/maglev/maglev-code-generator.cc | 16 ---------------- src/maglev/x64/maglev-assembler-x64-inl.h | 3 +-- src/maglev/x64/maglev-assembler-x64.cc | 5 ++--- src/maglev/x64/maglev-ir-x64.cc | 8 -------- 9 files changed, 4 insertions(+), 46 deletions(-) diff --git a/src/codegen/arm64/macro-assembler-arm64.cc b/src/codegen/arm64/macro-assembler-arm64.cc index 956e0ae434..bd8b6561ea 100644 --- a/src/codegen/arm64/macro-assembler-arm64.cc +++ b/src/codegen/arm64/macro-assembler-arm64.cc @@ -1589,13 +1589,6 @@ void MacroAssembler::AssertNotSmi(Register object, AbortReason reason) { Check(ne, reason); } -void TurboAssembler::AssertZeroExtended(Register int32_register) { - if (!v8_flags.debug_code) return; - ASM_CODE_COMMENT(this); - Tst(int32_register.X(), kMaxUInt32); - Check(ls, AbortReason::k32BitValueInRegisterIsNotZeroExtended); -} - void MacroAssembler::AssertCodeT(Register object) { if (!v8_flags.debug_code) return; ASM_CODE_COMMENT(this); diff --git a/src/codegen/arm64/macro-assembler-arm64.h b/src/codegen/arm64/macro-assembler-arm64.h index 6b5fb09add..e5f87cb025 100644 --- a/src/codegen/arm64/macro-assembler-arm64.h +++ b/src/codegen/arm64/macro-assembler-arm64.h @@ -1910,10 +1910,6 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler { AbortReason reason = AbortReason::kOperandIsASmi) NOOP_UNLESS_DEBUG_CODE - // Abort execution if a 64 bit register containing a 32 bit payload does - // not have zeros in the top 32 bits, enabled via --debug-code. - void AssertZeroExtended(Register int32_register) NOOP_UNLESS_DEBUG_CODE - // Abort execution if argument is not a CodeT, enabled via --debug-code. void AssertCodeT(Register object) NOOP_UNLESS_DEBUG_CODE diff --git a/src/maglev/arm64/maglev-assembler-arm64-inl.h b/src/maglev/arm64/maglev-assembler-arm64-inl.h index 2f5fa4bac9..961dbe127d 100644 --- a/src/maglev/arm64/maglev-assembler-arm64-inl.h +++ b/src/maglev/arm64/maglev-assembler-arm64-inl.h @@ -439,7 +439,7 @@ inline void MaglevAssembler::Move(Register dst, TaggedIndex i) { Mov(dst, i.ptr()); } inline void MaglevAssembler::Move(Register dst, int32_t i) { - Mov(dst.W(), Immediate(i)); + Mov(dst, Immediate(i)); } inline void MaglevAssembler::Move(DoubleRegister dst, double n) { Fmov(dst, n); diff --git a/src/maglev/arm64/maglev-assembler-arm64.cc b/src/maglev/arm64/maglev-assembler-arm64.cc index 2cb5198a4c..0363222d47 100644 --- a/src/maglev/arm64/maglev-assembler-arm64.cc +++ b/src/maglev/arm64/maglev-assembler-arm64.cc @@ -371,7 +371,6 @@ void MaglevAssembler::StringFromCharCode(RegisterSnapshot register_snapshot, Label* char_code_fits_one_byte, Register result, Register char_code, Register scratch) { - AssertZeroExtended(char_code); DCHECK_NE(char_code, scratch); ZoneLabelRef done(this); Cmp(char_code, Immediate(String::kMaxOneByteCharCode)); @@ -579,8 +578,6 @@ void MaglevAssembler::TruncateDoubleToInt32(Register dst, DoubleRegister src) { src, dst, done); Bind(*done); - // Zero extend the converted value to complete the truncation. - Mov(dst, Operand(dst.W(), UXTW)); } } // namespace maglev diff --git a/src/maglev/maglev-assembler.h b/src/maglev/maglev-assembler.h index 20f16cb5c9..61922dae33 100644 --- a/src/maglev/maglev-assembler.h +++ b/src/maglev/maglev-assembler.h @@ -7,9 +7,7 @@ #include "src/codegen/machine-type.h" #include "src/codegen/macro-assembler.h" -#include "src/flags/flags.h" #include "src/maglev/maglev-code-gen-state.h" -#include "src/maglev/maglev-ir.h" namespace v8 { namespace internal { diff --git a/src/maglev/maglev-code-generator.cc b/src/maglev/maglev-code-generator.cc index 4141d28ef4..24d2465cb6 100644 --- a/src/maglev/maglev-code-generator.cc +++ b/src/maglev/maglev-code-generator.cc @@ -638,22 +638,6 @@ class MaglevCodeGeneratingNodeProcessor { state); } - if (v8_flags.debug_code) { - // Check that all int32/uint32 inputs are zero extended - for (Input& input : *node) { - ValueRepresentation rep = - input.node()->properties().value_representation(); - if (rep == ValueRepresentation::kInt32 || - rep == ValueRepresentation::kUint32) { - // TODO(leszeks): Ideally we'd check non-register inputs too, but - // AssertZeroExtended needs the scratch register, so we'd have to do - // some manual push/pop here to free up another register. - if (input.IsGeneralRegister()) { - __ AssertZeroExtended(ToRegister(input)); - } - } - } - } node->GenerateCode(masm(), state); if (std::is_base_of::value) { diff --git a/src/maglev/x64/maglev-assembler-x64-inl.h b/src/maglev/x64/maglev-assembler-x64-inl.h index d3450ba1b4..12be9923d6 100644 --- a/src/maglev/x64/maglev-assembler-x64-inl.h +++ b/src/maglev/x64/maglev-assembler-x64-inl.h @@ -325,8 +325,7 @@ inline void MaglevAssembler::Move(Register dst, Register src) { } inline void MaglevAssembler::Move(Register dst, int32_t i) { - // Move as a uint32 to avoid sign extension. - MacroAssembler::Move(dst, static_cast(i)); + MacroAssembler::Move(dst, Immediate(i)); } inline void MaglevAssembler::Move(DoubleRegister dst, double n) { diff --git a/src/maglev/x64/maglev-assembler-x64.cc b/src/maglev/x64/maglev-assembler-x64.cc index 2c1a79b001..b8073b960c 100644 --- a/src/maglev/x64/maglev-assembler-x64.cc +++ b/src/maglev/x64/maglev-assembler-x64.cc @@ -100,7 +100,8 @@ void MaglevAssembler::AllocateTwoByteString(RegisterSnapshot register_snapshot, void MaglevAssembler::LoadSingleCharacterString(Register result, Register char_code, Register scratch) { - AssertZeroExtended(char_code); + // Make sure char_code is zero extended. + movl(char_code, char_code); if (v8_flags.debug_code) { cmpq(char_code, Immediate(String::kMaxOneByteCharCode)); Assert(below_equal, AbortReason::kUnexpectedValue); @@ -364,8 +365,6 @@ void MaglevAssembler::TruncateDoubleToInt32(Register dst, DoubleRegister src) { }, src, dst, done); bind(*done); - // Zero extend the converted value to complete the truncation. - movl(dst, dst); } void MaglevAssembler::Prologue(Graph* graph) { diff --git a/src/maglev/x64/maglev-ir-x64.cc b/src/maglev/x64/maglev-ir-x64.cc index 63ba42ec03..1b21c869d5 100644 --- a/src/maglev/x64/maglev-ir-x64.cc +++ b/src/maglev/x64/maglev-ir-x64.cc @@ -792,10 +792,6 @@ void LoadTaggedElement::GenerateCode(MaglevAssembler* masm, __ DecompressAnyTagged(kScratchRegister, FieldOperand(object, JSObject::kElementsOffset)); } - if (v8_flags.debug_code) { - __ cmpq(index, Immediate(0)); - __ Assert(above_equal, AbortReason::kUnexpectedNegativeValue); - } __ DecompressAnyTagged( result_reg, FieldOperand(kScratchRegister, index, times_tagged_size, FixedArray::kHeaderSize)); @@ -826,10 +822,6 @@ void LoadDoubleElement::GenerateCode(MaglevAssembler* masm, __ DecompressAnyTagged(kScratchRegister, FieldOperand(object, JSObject::kElementsOffset)); } - if (v8_flags.debug_code) { - __ cmpq(index, Immediate(0)); - __ Assert(above_equal, AbortReason::kUnexpectedNegativeValue); - } __ Movsd(result_reg, FieldOperand(kScratchRegister, index, times_8, FixedDoubleArray::kHeaderSize)); }