Skip to content

Commit 559ba38

Browse files
Nicoshevmeta-codesync[bot]
authored andcommitted
Improve SparseMaskIter on Aarch64
Summary: Instruction CTZ is not available on armv9a CPUs. This implies that to compute trailing zeroes, RBIT followed by CLZ must be issued. We are changing the mask's logic to reverse bits once nad rely on CLZ instead of CTZ. The former instruction sequence looked like this: 2c3e48: fmov x15, d1 2c3e4c: rbit x16, x15 2c3e50: clz x16, x16 2c3e54: lsl x16, x16, #1 2c3e58: and x16, x16, #0xf8 2c3e5c: ldr w16, [x14, x16] 2c3e60: cbz w16, 2c3e88 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x35c> 2c3e64: sub x16, x15, #0x1 2c3e68: ands x15, x16, x15 2c3e6c: b.ne 2c3e4c <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x320> // b.any The newer looks like this: 2c3d90: fmov x15, d1 2c3d94: rbit x15, x15 2c3d98: clz x17, x15 2c3d9c: lsl x18, x17, #1 2c3da0: and x18, x18, #0xf8 2c3da4: ldr w18, [x16, x18] 2c3da8: cbz w18, 2c3dd0 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x364> 2c3dac: lsr x17, x12, x17 2c3db0: ands x15, x17, x15 2c3db4: b.ne 2c3d98 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x32c> // b.any We can observe three things: -The final conditional branch jumps back to clz instead of rbit. -Instruction lsr depends on the result of clz, whereas sub depends on the result of rbit; meaning the old codepath can be executed speculatively earlier. -Assignment of 0x7FFFFFFFFFFFFFFF has been hoisted. There are no improvements nor regressions observed on benchmarks, probably it doesn't hit the case where many matches occur within the same tag. The added instruction of assigning 0x7FFFFFFFFFFFFFFF could potentially delay the tag memory load by 1 cycle, although it's unlikely This change allows performance improvements on occupiedIter: D94023144 Reviewed By: yfeldblum Differential Revision: D94020004 fbshipit-source-id: 8f8462f0290615fc8606e5ab9e3366a2a0733c51
1 parent 9b3e5ec commit 559ba38

1 file changed

Lines changed: 20 additions & 2 deletions

File tree

  • third-party/folly/src/folly/container/detail

third-party/folly/src/folly/container/detail/F14Mask.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ FOLLY_ALWAYS_INLINE static unsigned findFirstSetNonZero(T mask) {
4343
}
4444
}
4545

46+
template <typename T>
47+
FOLLY_ALWAYS_INLINE static unsigned findLastSetNonZero(T mask) {
48+
assume(mask != 0);
49+
if (sizeof(mask) == sizeof(unsigned)) {
50+
return __builtin_clz(static_cast<unsigned>(mask));
51+
} else {
52+
return __builtin_clzll(mask);
53+
}
54+
}
55+
4656
#if FOLLY_NEON
4757
using MaskType = uint64_t;
4858

@@ -130,14 +140,22 @@ class SparseMaskIter {
130140
MaskType mask_;
131141

132142
public:
143+
144+
#if FOLLY_AARCH64
145+
explicit SparseMaskIter(MaskType mask) : mask_{bitReverse(mask)} {}
146+
#else
133147
explicit SparseMaskIter(MaskType mask) : mask_{mask} {}
148+
#endif
134149

135150
bool hasNext() { return mask_ != 0; }
136151

137152
unsigned next() {
138153
FOLLY_SAFE_DCHECK(hasNext(), "");
139-
unsigned i = findFirstSetNonZero(mask_);
140-
mask_ &= (mask_ - 1);
154+
constexpr uint64_t lo63 = 0x7FFFFFFFFFFFFFFFull;
155+
static_assert(lo63 == (~0ull >> 1));
156+
unsigned i =
157+
kIsArchAArch64 ? findLastSetNonZero(mask_) : findFirstSetNonZero(mask_);
158+
mask_ &= kIsArchAArch64 ? (lo63 >> i) : (mask_ - 1);
141159
return i / kMaskSpacing;
142160
}
143161
};

0 commit comments

Comments
 (0)