From 6e1293b5f7142552f87c3ad84efa7de64dbea418 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 9 Jan 2024 01:48:31 +0000 Subject: [PATCH] Bug 1718516 - Fix AddToHash of 64-bits numbers on 32-bits platforms. r=nika,anba Differential Revision: https://phabricator.services.mozilla.com/D197302 --- js/src/jit/MacroAssembler.cpp | 15 +-------------- mfbt/HashFunctions.h | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index a62ea0f61395..d6f7de6c358b 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -7639,18 +7639,7 @@ void MacroAssembler::prepareHashNonGCThing(ValueOperand value, Register result, move64(value.toRegister64(), r64); rshift64Arithmetic(Imm32(32), r64); #else - // TODO: This seems like a bug in mozilla::detail::AddUintptrToHash(). - // The uint64_t input is first converted to uintptr_t and then back to - // uint64_t. But |uint64_t(uintptr_t(bits))| actually only clears the high - // bits, so this computation: - // - // aValue = uintptr_t(bits) - // v2 = static_cast(static_cast(aValue) >> 32) - // - // really just sets |v2 = 0|. And that means the xor-operation in AddU32ToHash - // can be optimized away, because |x ^ 0 = x|. - // - // Filed as bug 1718516. + move32(value.typeReg(), temp); #endif // mozilla::WrappingMultiply(kGoldenRatioU32, RotateLeft5(aHash) ^ aValue); @@ -7660,9 +7649,7 @@ void MacroAssembler::prepareHashNonGCThing(ValueOperand value, Register result, // mozilla::WrappingMultiply(kGoldenRatioU32, RotateLeft5(aHash) ^ aValue); // with |aHash = | and |aValue = v2|. rotateLeft(Imm32(5), result, result); -#ifdef JS_PUNBOX64 xor32(temp, result); -#endif // Combine |mul32| and |scrambleHashCode| by directly multiplying with // |kGoldenRatioU32 * kGoldenRatioU32|. diff --git a/mfbt/HashFunctions.h b/mfbt/HashFunctions.h index 4b740a3db16b..b9c2d5e98d7a 100644 --- a/mfbt/HashFunctions.h +++ b/mfbt/HashFunctions.h @@ -153,17 +153,20 @@ constexpr HashNumber AddU32ToHash(HashNumber aHash, uint32_t aValue) { } /** - * AddUintptrToHash takes sizeof(uintptr_t) as a template parameter. + * AddUintNToHash takes sizeof(int_type) as a template parameter. + * Changes to these functions need to be propagated to + * MacroAssembler::prepareHashNonGCThing, which inlines them manually for + * the JIT. */ -template -constexpr HashNumber AddUintptrToHash(HashNumber aHash, uintptr_t aValue) { +template +constexpr HashNumber AddUintNToHash(HashNumber aHash, uint64_t aValue) { return AddU32ToHash(aHash, static_cast(aValue)); } template <> -inline HashNumber AddUintptrToHash<8>(HashNumber aHash, uintptr_t aValue) { +inline HashNumber AddUintNToHash<8>(HashNumber aHash, uint64_t aValue) { uint32_t v1 = static_cast(aValue); - uint32_t v2 = static_cast(static_cast(aValue) >> 32); + uint32_t v2 = static_cast(aValue >> 32); return AddU32ToHash(AddU32ToHash(aHash, v1), v2); } @@ -196,23 +199,23 @@ template static_assert(sizeof(aA) == sizeof(uintptr_t), "Strange pointer!"); - return detail::AddUintptrToHash(aHash, uintptr_t(aA)); + return detail::AddUintNToHash(aHash, uintptr_t(aA)); } -// We use AddUintptrToHash() for hashing all integral types. 8-byte integral +// We use AddUintNToHash() for hashing all integral types. 8-byte integral // types are treated the same as 64-bit pointers, and smaller integral types are -// first implicitly converted to 32 bits and then passed to AddUintptrToHash() +// first implicitly converted to 32 bits and then passed to AddUintNToHash() // to be hashed. template , int> = 0> [[nodiscard]] constexpr HashNumber AddToHash(HashNumber aHash, T aA) { - return detail::AddUintptrToHash(aHash, aA); + return detail::AddUintNToHash(aHash, aA); } template , int> = 0> [[nodiscard]] constexpr HashNumber AddToHash(HashNumber aHash, T aA) { - // Hash using AddUintptrToHash with the underlying type of the enum type + // Hash using AddUintNToHash with the underlying type of the enum type using UnderlyingType = typename std::underlying_type::type; - return detail::AddUintptrToHash( + return detail::AddUintNToHash( aHash, static_cast(aA)); }