Skip to content

Commit 0ffb482

Browse files
committed
Unreviewed, relanding with TestWTF fix
https://bugs.webkit.org/show_bug.cgi?id=309200 rdar://171758273 Zero is empty key, so you need to use WTF::UnsignedWithZeroKeyHashTraits explicitly. Test: Tools/TestWebKitAPI/Tests/WTF/SmallSet.cpp * Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp: * Source/JavaScriptCore/b3/air/AirCode.h: * Source/WTF/wtf/SmallSet.h: * Tools/TestWebKitAPI/Tests/WTF/SmallSet.cpp: (TestWebKitAPI::testSmallSetOfUnsigned): (TestWebKitAPI::testVectorsOfSmallSetsOfUnsigned): Canonical link: https://commits.webkit.org/308679@main
1 parent 381ad0c commit 0ffb482

File tree

4 files changed

+27
-37
lines changed

4 files changed

+27
-37
lines changed

Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ class AbstractColoringAllocator {
486486
Vector<Vector<IndexType, 0, UnsafeVectorOverflow, 4>, 0, UnsafeVectorOverflow> m_adjacencyList;
487487
Vector<IndexType, 0, UnsafeVectorOverflow> m_degrees;
488488

489-
using IndexTypeSet = SmallSet<IndexType, IntHash<IndexType>>;
489+
using IndexTypeSet = SmallSet<IndexType, IntHash<IndexType>, WTF::UnsignedWithZeroKeyHashTraits<IndexType>>;
490490

491491
UncheckedKeyHashMap<IndexType, IndexTypeSet, DefaultHash<IndexType>, WTF::UnsignedWithZeroKeyHashTraits<IndexType>> m_biases;
492492

@@ -499,7 +499,7 @@ class AbstractColoringAllocator {
499499
Vector<MoveOperands, 0, UnsafeVectorOverflow> m_coalescingCandidates;
500500

501501
// List of every move instruction associated with a Tmp.
502-
Vector<SmallSet<unsigned, IntHash<unsigned>>> m_moveList;
502+
Vector<SmallSet<unsigned, IntHash<unsigned>, WTF::UnsignedWithZeroKeyHashTraits<unsigned>>> m_moveList;
503503

504504
// Colors.
505505
Vector<Reg, 0, UnsafeVectorOverflow> m_coloredTmp;

Source/JavaScriptCore/b3/air/AirCode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ class Code {
396396
Vector<std::unique_ptr<BasicBlock>> m_blocks;
397397
SparseCollection<Special> m_specials;
398398
std::unique_ptr<CFG> m_cfg;
399-
SmallSet<Tmp, DefaultHash<Tmp>, 2> m_fastTmps;
399+
SmallSet<Tmp, DefaultHash<Tmp>, HashTraits<Tmp>, 2> m_fastTmps;
400400
CCallSpecial* m_cCallSpecial { nullptr };
401401
unsigned m_numGPTmps { 0 };
402402
unsigned m_numFPTmps { 0 };

Source/WTF/wtf/SmallSet.h

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <wtf/Assertions.h>
3030
#include <wtf/FastMalloc.h>
3131
#include <wtf/HashFunctions.h>
32+
#include <wtf/HashTraits.h>
3233
#include <wtf/MallocSpan.h>
3334
#include <wtf/Noncopyable.h>
3435
#include <wtf/StdLibExtras.h>
@@ -40,10 +41,9 @@ DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(SmallSet);
4041
// Functionally, this class is very similar to Variant<Vector<T, SmallArraySize>, HashSet<T>>
4142
// It is optimized primarily for space, but is also quite fast
4243
// Its main limitation is that it has no way to remove elements once they have been added to it
43-
// Also, instead of being fully parameterized by a HashTrait parameter, it always uses -1 (all ones) as its empty value
44-
// Relatedly, it can only store objects of up to 64 bit size (but that particular limitation should be fairly easy to lift if needed)
44+
// It uses HashTraits to determine its empty value.
4545
// Use it whenever you need to store an unbounded but probably small number of unsigned integers or pointers.
46-
template<typename T, typename Hash = PtrHashBase<T, false /* isSmartPtr */>, unsigned SmallArraySize = 8>
46+
template<typename T, typename Hash = PtrHashBase<T, false /* isSmartPtr */>, typename Traits = HashTraits<T>, unsigned SmallArraySize = 8>
4747
class SmallSet {
4848
WTF_DEPRECATED_MAKE_FAST_ALLOCATED(SmallSet);
4949
WTF_MAKE_NONCOPYABLE(SmallSet);
@@ -101,7 +101,7 @@ class SmallSet {
101101
{
102102
++m_index;
103103
ASSERT(m_index <= m_buffer.size());
104-
while (m_index < m_buffer.size() && m_buffer[m_index] == emptyValue())
104+
while (m_index < m_buffer.size() && isEmptyBucket(m_buffer[m_index]))
105105
++m_index;
106106
return *this;
107107
}
@@ -111,7 +111,7 @@ class SmallSet {
111111
bool operator==(const iterator& other) const { ASSERT(m_buffer.data() == other.m_buffer.data()); return m_index == other.m_index; }
112112

113113
private:
114-
template<typename U, typename H, unsigned S> friend class WTF::SmallSet;
114+
template<typename U, typename H, typename TR, unsigned S> friend class WTF::SmallSet;
115115
unsigned m_index;
116116
std::span<T> m_buffer;
117117
};
@@ -203,11 +203,9 @@ class SmallSet {
203203
}
204204

205205
private:
206-
constexpr static T emptyValue()
206+
static bool isEmptyBucket(const T& value)
207207
{
208-
if constexpr (std::is_pointer<T>::value)
209-
return static_cast<T>(std::bit_cast<void*>(std::numeric_limits<uintptr_t>::max()));
210-
return std::numeric_limits<T>::max();
208+
return isHashTraitsEmptyValue<Traits>(value);
211209
}
212210

213211
bool equal(const T left, const T right) const
@@ -216,14 +214,12 @@ class SmallSet {
216214
return Hash::equal(left, right);
217215
if (isValidEntry(left) && isValidEntry(right))
218216
return Hash::equal(left, right);
219-
return left == right;
217+
return left == right;
220218
}
221219

222-
bool isValidEntry(const T value) const
220+
bool isValidEntry(const T& value) const
223221
{
224-
if constexpr (Hash::safeToCompareToEmptyOrDeleted)
225-
return !Hash::equal(value, emptyValue());
226-
return value != emptyValue();
222+
return !isEmptyBucket(value);
227223
}
228224

229225
inline bool isSmall() const
@@ -235,34 +231,28 @@ class SmallSet {
235231
{
236232
m_size = 0;
237233
m_capacity = SmallArraySize;
238-
memsetSpan(std::span { m_inline.smallStorage }, -1);
234+
initializeBuckets(std::span { m_inline.smallStorage });
239235
ASSERT(isSmall());
240236
}
241237

242-
inline void grow(unsigned size)
238+
static void initializeBuckets(std::span<T> span)
243239
{
244-
// We memset the new buffer with -1, so for consistency emptyValue() must return something which is all 1s.
245-
#if !defined(NDEBUG)
246-
if constexpr (std::is_pointer<T>::value)
247-
ASSERT(std::bit_cast<intptr_t>(emptyValue()) == -1ll);
248-
else if constexpr (sizeof(T) == 8)
249-
ASSERT(std::bit_cast<int64_t>(emptyValue()) == -1ll);
250-
else if constexpr (sizeof(T) == 4)
251-
ASSERT(std::bit_cast<int32_t>(emptyValue()) == -1);
252-
else if constexpr (sizeof(T) == 2)
253-
ASSERT(std::bit_cast<int16_t>(emptyValue()) == -1);
254-
else if constexpr (sizeof(T) == 1)
255-
ASSERT(std::bit_cast<int8_t>(emptyValue()) == -1);
256-
else
257-
RELEASE_ASSERT_NOT_REACHED();
258-
#endif
240+
if constexpr (Traits::emptyValueIsZero)
241+
memsetSpan(span, 0);
242+
else {
243+
for (auto& entry : span)
244+
entry = Traits::emptyValue();
245+
}
246+
}
259247

248+
inline void grow(unsigned size)
249+
{
260250
size_t allocationSize = sizeof(T) * size;
261251
auto oldBuffer = buffer();
262252

263253
unsigned oldCapacity = m_capacity;
264254
auto newBuffer = MallocSpan<T, SmallSetMalloc>::malloc(allocationSize);
265-
memsetSpan(newBuffer.mutableSpan(), -1);
255+
initializeBuckets(newBuffer.mutableSpan());
266256
m_capacity = size;
267257

268258
for (unsigned i = 0; i < oldCapacity; i++) {

Tools/TestWebKitAPI/Tests/WTF/SmallSet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ namespace TestWebKitAPI {
3838
template<typename T>
3939
void testSmallSetOfUnsigned(unsigned n)
4040
{
41-
SmallSet<T, IntHash<T>> set;
41+
SmallSet<T, IntHash<T>, WTF::UnsignedWithZeroKeyHashTraits<T>> set;
4242

4343
EXPECT_TRUE(set.isEmpty());
4444
EXPECT_EQ(set.size(), 0u);
@@ -107,7 +107,7 @@ void testSmallSetOfPointers()
107107
template<typename T>
108108
void testVectorsOfSmallSetsOfUnsigned()
109109
{
110-
Vector<SmallSet<T, IntHash<T>>, 2> vector;
110+
Vector<SmallSet<T, IntHash<T>, WTF::UnsignedWithZeroKeyHashTraits<T>>, 2> vector;
111111
vector.append({ });
112112
vector.append({ });
113113

0 commit comments

Comments
 (0)