From 7cb82a76b46ef18f8e05d40d4fe13b3410d446e4 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 15 Sep 2014 10:54:49 +0000 Subject: [PATCH] Reland "Remove V8_HOST_CAN_READ_UNALIGNED and its uses." BUG=chromium:412967 LOG=N R=jkummerow@chromium.org Review URL: https://codereview.chromium.org/571903002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23938 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/base/build_config.h | 4 -- src/deoptimizer.h | 10 ++- src/objects.cc | 31 +--------- src/objects.h | 27 +++++--- src/regexp-macro-assembler-irregexp.h | 1 + src/regexp-macro-assembler.cc | 9 --- src/regexp-macro-assembler.h | 2 +- src/runtime.cc | 61 ++++++++++--------- src/snapshot-source-sink.cc | 4 -- src/utils.h | 25 ++------ test/mjsunit/regress/string-compare-memcmp.js | 7 +++ 11 files changed, 71 insertions(+), 110 deletions(-) create mode 100644 test/mjsunit/regress/string-compare-memcmp.js diff --git a/src/base/build_config.h b/src/base/build_config.h index 22b944be9..2bf57c963 100644 --- a/src/base/build_config.h +++ b/src/base/build_config.h @@ -21,7 +21,6 @@ // V8_HOST_ARCH_IA32 on both 32- and 64-bit x86. #define V8_HOST_ARCH_IA32 1 #define V8_HOST_ARCH_32_BIT 1 -#define V8_HOST_CAN_READ_UNALIGNED 1 #else #define V8_HOST_ARCH_X64 1 #if defined(__x86_64__) && __SIZEOF_POINTER__ == 4 // Check for x32. @@ -29,16 +28,13 @@ #else #define V8_HOST_ARCH_64_BIT 1 #endif -#define V8_HOST_CAN_READ_UNALIGNED 1 #endif // __native_client__ #elif defined(_M_IX86) || defined(__i386__) #define V8_HOST_ARCH_IA32 1 #define V8_HOST_ARCH_32_BIT 1 -#define V8_HOST_CAN_READ_UNALIGNED 1 #elif defined(__AARCH64EL__) #define V8_HOST_ARCH_ARM64 1 #define V8_HOST_ARCH_64_BIT 1 -#define V8_HOST_CAN_READ_UNALIGNED 1 #elif defined(__ARMEL__) #define V8_HOST_ARCH_ARM 1 #define V8_HOST_ARCH_32_BIT 1 diff --git a/src/deoptimizer.h b/src/deoptimizer.h index 8b4b26397..4becf6e30 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -17,19 +17,17 @@ namespace internal { static inline double read_double_value(Address p) { -#ifdef V8_HOST_CAN_READ_UNALIGNED - return Memory::double_at(p); -#else // V8_HOST_CAN_READ_UNALIGNED // Prevent gcc from using load-double (mips ldc1) on (possibly) // non-64-bit aligned address. + // We assume that the address is 32-bit aligned. + DCHECK(IsAligned(reinterpret_cast(p), kInt32Size)); union conversion { double d; uint32_t u[2]; } c; - c.u[0] = *reinterpret_cast(p); - c.u[1] = *reinterpret_cast(p + 4); + c.u[0] = Memory::uint32_at(p); + c.u[1] = Memory::uint32_at(p + 4); return c.d; -#endif // V8_HOST_CAN_READ_UNALIGNED } diff --git a/src/objects.cc b/src/objects.cc index e123bc208..13688c780 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -8507,36 +8507,7 @@ template static inline bool CompareRawStringContents(const Char* const a, const Char* const b, int length) { - int i = 0; -#ifndef V8_HOST_CAN_READ_UNALIGNED - // If this architecture isn't comfortable reading unaligned ints - // then we have to check that the strings are aligned before - // comparing them blockwise. - const int kAlignmentMask = sizeof(uint32_t) - 1; // NOLINT - uintptr_t pa_addr = reinterpret_cast(a); - uintptr_t pb_addr = reinterpret_cast(b); - if (((pa_addr & kAlignmentMask) | (pb_addr & kAlignmentMask)) == 0) { -#endif - const int kStepSize = sizeof(int) / sizeof(Char); // NOLINT - int endpoint = length - kStepSize; - // Compare blocks until we reach near the end of the string. - for (; i <= endpoint; i += kStepSize) { - uint32_t wa = *reinterpret_cast(a + i); - uint32_t wb = *reinterpret_cast(b + i); - if (wa != wb) { - return false; - } - } -#ifndef V8_HOST_CAN_READ_UNALIGNED - } -#endif - // Compare the remaining characters that didn't fit into a block. - for (; i < length; i++) { - if (a[i] != b[i]) { - return false; - } - } - return true; + return CompareChars(a, b, length) == 0; } diff --git a/src/objects.h b/src/objects.h index 32dd94f39..8203b36a9 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9172,22 +9172,33 @@ class String: public Name { static inline int NonAsciiStart(const char* chars, int length) { const char* start = chars; const char* limit = chars + length; -#ifdef V8_HOST_CAN_READ_UNALIGNED - DCHECK(unibrow::Utf8::kMaxOneByteChar == 0x7F); - const uintptr_t non_one_byte_mask = kUintptrAllBitsSet / 0xFF * 0x80; - while (chars + sizeof(uintptr_t) <= limit) { - if (*reinterpret_cast(chars) & non_one_byte_mask) { - return static_cast(chars - start); + + if (length >= kIntptrSize) { + // Check unaligned bytes. + while (!IsAligned(reinterpret_cast(chars), sizeof(uintptr_t))) { + if (static_cast(*chars) > unibrow::Utf8::kMaxOneByteChar) { + return static_cast(chars - start); + } + ++chars; + } + // Check aligned words. + DCHECK(unibrow::Utf8::kMaxOneByteChar == 0x7F); + const uintptr_t non_one_byte_mask = kUintptrAllBitsSet / 0xFF * 0x80; + while (chars + sizeof(uintptr_t) <= limit) { + if (*reinterpret_cast(chars) & non_one_byte_mask) { + return static_cast(chars - start); + } + chars += sizeof(uintptr_t); } - chars += sizeof(uintptr_t); } -#endif + // Check remaining unaligned bytes. while (chars < limit) { if (static_cast(*chars) > unibrow::Utf8::kMaxOneByteChar) { return static_cast(chars - start); } ++chars; } + return static_cast(chars - start); } diff --git a/src/regexp-macro-assembler-irregexp.h b/src/regexp-macro-assembler-irregexp.h index cdfb46ad1..b192c22b6 100644 --- a/src/regexp-macro-assembler-irregexp.h +++ b/src/regexp-macro-assembler-irregexp.h @@ -31,6 +31,7 @@ class RegExpMacroAssemblerIrregexp: public RegExpMacroAssembler { virtual ~RegExpMacroAssemblerIrregexp(); // The byte-code interpreter checks on each push anyway. virtual int stack_limit_slack() { return 1; } + virtual bool CanReadUnaligned() { return false; } virtual void Bind(Label* label); virtual void AdvanceCurrentPosition(int by); // Signed cp change. virtual void PopCurrentPosition(); diff --git a/src/regexp-macro-assembler.cc b/src/regexp-macro-assembler.cc index c4bfc8d7f..52df648d9 100644 --- a/src/regexp-macro-assembler.cc +++ b/src/regexp-macro-assembler.cc @@ -24,15 +24,6 @@ RegExpMacroAssembler::~RegExpMacroAssembler() { } -bool RegExpMacroAssembler::CanReadUnaligned() { -#ifdef V8_HOST_CAN_READ_UNALIGNED - return true; -#else - return false; -#endif -} - - #ifndef V8_INTERPRETED_REGEXP // Avoid unused code, e.g., on ARM. NativeRegExpMacroAssembler::NativeRegExpMacroAssembler(Zone* zone) diff --git a/src/regexp-macro-assembler.h b/src/regexp-macro-assembler.h index 6bb411556..f72cc4d42 100644 --- a/src/regexp-macro-assembler.h +++ b/src/regexp-macro-assembler.h @@ -48,7 +48,7 @@ class RegExpMacroAssembler { // kCheckStackLimit flag to push operations (instead of kNoStackLimitCheck) // at least once for every stack_limit() pushes that are executed. virtual int stack_limit_slack() = 0; - virtual bool CanReadUnaligned(); + virtual bool CanReadUnaligned() = 0; virtual void AdvanceCurrentPosition(int by) = 0; // Signed cp change. virtual void AdvanceRegister(int reg, int by) = 0; // r[reg] += by. // Continues execution from the position pushed on the top of the backtrack diff --git a/src/runtime.cc b/src/runtime.cc index 20f311089..67f749b27 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -6552,34 +6552,38 @@ static bool FastAsciiConvert(char* dst, bool changed = false; uintptr_t or_acc = 0; const char* const limit = src + length; -#ifdef V8_HOST_CAN_READ_UNALIGNED - // Process the prefix of the input that requires no conversion one - // (machine) word at a time. - while (src <= limit - sizeof(uintptr_t)) { - const uintptr_t w = *reinterpret_cast(src); - or_acc |= w; - if (AsciiRangeMask(w, lo, hi) != 0) { - changed = true; - break; + + // dst is newly allocated and always aligned. + DCHECK(IsAligned(reinterpret_cast(dst), sizeof(uintptr_t))); + // Only attempt processing one word at a time if src is also aligned. + if (IsAligned(reinterpret_cast(src), sizeof(uintptr_t))) { + // Process the prefix of the input that requires no conversion one aligned + // (machine) word at a time. + while (src <= limit - sizeof(uintptr_t)) { + const uintptr_t w = *reinterpret_cast(src); + or_acc |= w; + if (AsciiRangeMask(w, lo, hi) != 0) { + changed = true; + break; + } + *reinterpret_cast(dst) = w; + src += sizeof(uintptr_t); + dst += sizeof(uintptr_t); + } + // Process the remainder of the input performing conversion when + // required one word at a time. + while (src <= limit - sizeof(uintptr_t)) { + const uintptr_t w = *reinterpret_cast(src); + or_acc |= w; + uintptr_t m = AsciiRangeMask(w, lo, hi); + // The mask has high (7th) bit set in every byte that needs + // conversion and we know that the distance between cases is + // 1 << 5. + *reinterpret_cast(dst) = w ^ (m >> 2); + src += sizeof(uintptr_t); + dst += sizeof(uintptr_t); } - *reinterpret_cast(dst) = w; - src += sizeof(uintptr_t); - dst += sizeof(uintptr_t); - } - // Process the remainder of the input performing conversion when - // required one word at a time. - while (src <= limit - sizeof(uintptr_t)) { - const uintptr_t w = *reinterpret_cast(src); - or_acc |= w; - uintptr_t m = AsciiRangeMask(w, lo, hi); - // The mask has high (7th) bit set in every byte that needs - // conversion and we know that the distance between cases is - // 1 << 5. - *reinterpret_cast(dst) = w ^ (m >> 2); - src += sizeof(uintptr_t); - dst += sizeof(uintptr_t); } -#endif // Process the last few bytes of the input (or the whole input if // unaligned access is not supported). while (src < limit) { @@ -6593,9 +6597,8 @@ static bool FastAsciiConvert(char* dst, ++src; ++dst; } - if ((or_acc & kAsciiMask) != 0) { - return false; - } + + if ((or_acc & kAsciiMask) != 0) return false; DCHECK(CheckFastAsciiConvert( saved_dst, saved_src, length, changed, Converter::kIsToLower)); diff --git a/src/snapshot-source-sink.cc b/src/snapshot-source-sink.cc index 2be14383f..44f87060f 100644 --- a/src/snapshot-source-sink.cc +++ b/src/snapshot-source-sink.cc @@ -24,14 +24,10 @@ SnapshotByteSource::~SnapshotByteSource() { } int32_t SnapshotByteSource::GetUnalignedInt() { DCHECK(position_ < length_); // Require at least one byte left. -#if defined(V8_HOST_CAN_READ_UNALIGNED) && __BYTE_ORDER == __LITTLE_ENDIAN - int32_t answer = *reinterpret_cast(data_ + position_); -#else int32_t answer = data_[position_]; answer |= data_[position_ + 1] << 8; answer |= data_[position_ + 2] << 16; answer |= data_[position_ + 3] << 24; -#endif return answer; } diff --git a/src/utils.h b/src/utils.h index 07b6490bf..c23cf05f6 100644 --- a/src/utils.h +++ b/src/utils.h @@ -680,20 +680,11 @@ inline int CompareCharsUnsigned(const lchar* lhs, const rchar* rhs, int chars) { const lchar* limit = lhs + chars; -#ifdef V8_HOST_CAN_READ_UNALIGNED - if (sizeof(*lhs) == sizeof(*rhs)) { - // Number of characters in a uintptr_t. - static const int kStepSize = sizeof(uintptr_t) / sizeof(*lhs); // NOLINT - while (lhs <= limit - kStepSize) { - if (*reinterpret_cast(lhs) != - *reinterpret_cast(rhs)) { - break; - } - lhs += kStepSize; - rhs += kStepSize; - } + if (sizeof(*lhs) == sizeof(char) && sizeof(*rhs) == sizeof(char)) { + // memcmp compares byte-by-byte, yielding wrong results for two-byte + // strings on little-endian systems. + return memcmp(lhs, rhs, chars); } -#endif while (lhs < limit) { int r = static_cast(*lhs) - static_cast(*rhs); if (r != 0) return r; @@ -1286,15 +1277,11 @@ void CopyChars(sinkchar* dest, const sourcechar* src, int chars) { template void CopyCharsUnsigned(sinkchar* dest, const sourcechar* src, int chars) { sinkchar* limit = dest + chars; -#ifdef V8_HOST_CAN_READ_UNALIGNED if ((sizeof(*dest) == sizeof(*src)) && (chars >= static_cast(kMinComplexMemCopy / sizeof(*dest)))) { MemCopy(dest, src, chars * sizeof(*dest)); - return; - } -#endif - while (dest < limit) { - *dest++ = static_cast(*src++); + } else { + while (dest < limit) *dest++ = static_cast(*src++); } } diff --git a/test/mjsunit/regress/string-compare-memcmp.js b/test/mjsunit/regress/string-compare-memcmp.js new file mode 100644 index 000000000..45f47343e --- /dev/null +++ b/test/mjsunit/regress/string-compare-memcmp.js @@ -0,0 +1,7 @@ +// Copyright 2012 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +assertEquals(-1, %StringCompare("abc\u0102", "abc\u0201")); -- 2.34.1