Skip to content

Fix MGARD-X HIP Huffman lossless (codec correctness + code-length OOB)#257

Open
gqian-coder wants to merge 1 commit into
CODARcode:masterfrom
gqian-coder:hip-lossless-huffman-fixes
Open

Fix MGARD-X HIP Huffman lossless (codec correctness + code-length OOB)#257
gqian-coder wants to merge 1 commit into
CODARcode:masterfrom
gqian-coder:hip-lossless-huffman-fixes

Conversation

@gqian-coder

Copy link
Copy Markdown
Contributor

Four independent bugs in the MGARD-X HIP ParallelHuffman lossless path: three in the Huffman codec (canonical-codeword generation, header serialization, and codeword-length overflow handling) made GPU-compressed streams undecodable/oversized or abort, and one out-of-bounds device read in the parallel code-length generation faulted on the GPU. All verified on gfx90a (Frontier MI250X/MI210): HIP-compressed streams now round-trip and are byte-compatible with the SERIAL decoder.

HuffmanWorkspace.hpp: allocate status as non-managed device memory.
It was managed (hipMallocManaged); atomicMin on fine-grained memory is
unreliable on ROCm/gfx90a, so GenerateCW::Operation4's atomic min never
updated newCDPI. The Huffman length-group boundary was never found, all
symbols collapsed into one group, and the canonical codewords came out
with a constant offset -- codebook and decodebook mutually inconsistent,
so the stream was undecodable by any backend.

Huffman.hpp: default-initialize outlier_count = 0.
ComposedLosslessCompressor::Compress -> CompressPrimary never writes
outlier_count, but Serialize reads it to size compressed_data.resize().
The uninitialized value produced a garbage-sized allocation (hipMalloc
OOM, or an invalid Copy1D).

GetCodebook.hpp: throw instead of exit(1) when the longest codeword
exceeds the H-type budget (sizeof(H)*8 - 8 bits), so callers can catch
it and fall back to another lossless backend or a smaller dict_size.

GenerateCL.hpp: bounds-check histogram[_lNodesCur + _curLeavesNum] in the
parallel code-length generation; when every remaining leaf joins the
batch the index reached dict_size (one past end of the nz_dict_size
histogram), faulting on GPU.

Four independent bugs in the MGARD-X HIP ParallelHuffman lossless path:
three in the Huffman codec (canonical-codeword generation, header
serialization, and codeword-length overflow handling) made GPU-compressed
streams undecodable/oversized or abort, and one out-of-bounds device read
in the parallel code-length generation faulted on the GPU. All verified on
gfx90a (Frontier MI250X/MI210): HIP-compressed streams now round-trip and
are byte-compatible with the SERIAL decoder.

HuffmanWorkspace.hpp: allocate `status` as non-managed device memory.
  It was managed (hipMallocManaged); atomicMin on fine-grained memory is
  unreliable on ROCm/gfx90a, so GenerateCW::Operation4's atomic min never
  updated newCDPI. The Huffman length-group boundary was never found, all
  symbols collapsed into one group, and the canonical codewords came out
  with a constant offset -- codebook and decodebook mutually inconsistent,
  so the stream was undecodable by any backend.

Huffman.hpp: default-initialize `outlier_count = 0`.
  ComposedLosslessCompressor::Compress -> CompressPrimary never writes
  outlier_count, but Serialize reads it to size compressed_data.resize().
  The uninitialized value produced a garbage-sized allocation (hipMalloc
  OOM, or an invalid Copy1D).

GetCodebook.hpp: throw instead of exit(1) when the longest codeword
  exceeds the H-type budget (sizeof(H)*8 - 8 bits), so callers can catch
  it and fall back to another lossless backend or a smaller dict_size.

GenerateCL.hpp: bounds-check histogram[_lNodesCur + _curLeavesNum] in the
  parallel code-length generation; when every remaining leaf joins the
  batch the index reached dict_size (one past end of the nz_dict_size
  histogram), faulting on GPU.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gqian-coder
gqian-coder force-pushed the hip-lossless-huffman-fixes branch from ae8d8e6 to 177b1b3 Compare July 22, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants