Skip to content

Commit e820755

Browse files
Alexander Shturmmeta-codesync[bot]
authored andcommitted
Avoid ~mask in AVX-512 partition kernel (#965)
Summary: Pull Request resolved: #965 D116655191 synced the `pivco_huffman` codec into `openzl/prod`, and since then any `buck build //...` in fbcode stalls indefinitely on `openzl/prod:compress [c_compile src/openzl/codecs/pivco_huffman/arch/encode_pivco_avx512.c]`. Full analysis is on that diff in the Review Assistant panel; abridged here: - Under clang 19 at `-O3` the translation unit never finishes — I killed it at 10m28s with 2.1 GB RSS still climbing. Three `gdb` samples of the `cc1` process all land in SelectionDAG DAGCombine: `combineBitcastToBoolVector`, and `visitXOR` -> `haveNoCommonBitsSet` -> deeply recursive `computeKnownBits` on `v64i1`. - Source bisection pins it to `partitionImpl`, which complements a `__mmask64` (`~bits`, `~bits & valid`) while also round-tripping it through `uint64_t` for `ZL_writeLE64`. A `packFlatDepth`-only variant of the same file compiles in 0.35s. - openzl never saw this because `fbcode/openzl/PACKAGE` pins `llvm-fb: 21`, where the unmodified file compiles in 0.43s. A `//...` build reaches the target through an external dependent under `platform010-clang19-no-san`, bypassing the package pin. Deriving the left-child mask with a second comparison instead of complementing keeps LLVM 19 out of that combine loop and costs nothing: `_MM_CMPINT_LT` is the exact complement of `_MM_CMPINT_GE` on unsigned lanes, and in the tail `~(cmpGE & valid) & valid == cmpLT & valid`. The emitted bitstream is unchanged. Both trees are updated, since `sync_dev_to_prod.sh` mirrors dev into prod byte-for-byte. Reviewed By: kevinjzhang Differential Revision: D117300734 fbshipit-source-id: cc6e8213eac068842e393a92a8bfdb567e16b770
1 parent 01cf35a commit e820755

1 file changed

Lines changed: 13 additions & 7 deletions

File tree

src/openzl/codecs/pivco_huffman/arch/encode_pivco_avx512.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,14 @@ ZL_AVX512_INLINE size_t partitionImpl(
7272
ones += blockOnes;
7373
}
7474
if (kPartitionLhs) {
75-
// ~bits selects the left child; in a full 64-lane block every
76-
// inverted bit is a valid lane, so no extra masking is needed.
77-
_mm512_mask_compressstoreu_epi8(lhs + zeros, ~bits, rankVec);
75+
// A second comparison rather than ~bits: complementing a mask
76+
// register at -O3 drives LLVM 19's DAGCombiner into a
77+
// non-converging visitXOR/combineBitcastToBoolVector loop, so the
78+
// TU never finishes compiling. On unsigned lanes LT == ~GE.
79+
// TODO: T286065866 - revert once the codegen bug is fixed.
80+
const __mmask64 lhsBits =
81+
_mm512_cmp_epu8_mask(rankVec, threshold, _MM_CMPINT_LT);
82+
_mm512_mask_compressstoreu_epi8(lhs + zeros, lhsBits, rankVec);
7883
zeros += 64 - blockOnes;
7984
}
8085
}
@@ -96,10 +101,11 @@ ZL_AVX512_INLINE size_t partitionImpl(
96101
ones += blockOnes;
97102
}
98103
if (kPartitionLhs) {
99-
// For the tail, restrict the left child to valid lanes as well,
100-
// otherwise the padding lanes (which are 0-bits) would be stored.
101-
_mm512_mask_compressstoreu_epi8(
102-
lhs + zeros, ~bits & valid, rankVec);
104+
// Restricted to the valid lanes, otherwise the zeroed padding
105+
// lanes would compare LT and be stored.
106+
const __mmask64 lhsBits = _mm512_mask_cmp_epu8_mask(
107+
valid, rankVec, threshold, _MM_CMPINT_LT);
108+
_mm512_mask_compressstoreu_epi8(lhs + zeros, lhsBits, rankVec);
103109
zeros += lanes - blockOnes;
104110
}
105111
}

0 commit comments

Comments
 (0)