Conversation
The producer hands a block or a batch to a worker by writing its slot and setting run = TRUE, and reclaims the slot when it reads run == FALSE again. Both flags were volatile CMP_BOOL. volatile orders nothing between threads, so on a weakly ordered CPU the producer can observe a slot go idle before the worker's writes to *out are visible, then reuse the slot and overwrite in / batch_in while the worker still reads it. Measured on macOS arm64 before this change, encoding the same 512x512 image ten times: stock BC7 (OPTION_CMP_USE_BC7ENC_RDO=OFF) 10 distinct outputs of 10 bc7e batched 3 to 6 distinct of 10 One bc7e run lost 19dB of PSNR against the single-threaded reference. One run segfaulted. -NumThreads 1 was always correct, which points at the handoff rather than either codec. x86-64 store ordering hides this, which is why the Linux and Windows builds never showed it. It is not specific to the bc7e work: the stock BC7 codec uses the same worker pool and fails harder. run and exit are now std::atomic<bool>. The worker releases on store after writing *out; the producer acquires on load before reusing the slot; DispatchBatch releases after setting out and batch_count; and FinishBC7Encoding acquires so the caller sees every block the workers wrote. After the change all four configurations tested return one distinct output across twelve runs, matching -NumThreads 1 byte for byte.
Four independent defects, each of which stops a macOS CLI build or
corrupts what it writes.
C++ standard. The feature probe ran only for Linux and Windows hosts and
left Apple on C++11. That disables the std::filesystem branch in
cmp_fileio.cpp, so CMP_GetJustFileExt falls back to
substr(find_last_of('.') + 1) and returns "dds" rather than ".dds".
IsDestinationUnCompressed() compares against ".dds", so every
destination looked uncompressed, MidwayDecompress was set, and the CLI
wrote a decompressed DDS four times the expected size while reporting
success and exit 0. Apple clang implements C++17 and std::filesystem, so
Apple now takes the same branch as Windows. The C++14 branch stays for
Linux; it is skipped on Apple because its <experimental/filesystem> and
stdc++fs do not exist under libc++.
CPU detection. GetCPUID has no implementation outside Windows but left
the caller's four-int buffer untouched. Linux never notices because
GetCPUExtensions guards the whole probe with #ifndef __linux__, but
macOS enters it and read uninitialized stack, so the BC1 SIMD dispatch
could select an SSE, AVX or AVX-512 kernel on a CPU that does not
implement it. It now zero-fills, which reports no extensions and keeps
every non-Windows host on the scalar kernels the Linux build already
uses.
OpenCV. canalysis required the package unconditionally on non-Windows
hosts, so a CLI build failed to configure on any machine without OpenCV
installed even though OPTION_CMP_OPENCV=OFF already excludes ssim.cpp,
the only file that needs it. The find_package now follows the option.
Link libraries. The Apple branch hardcoded /usr/lib/libz.dylib, which
has had no file behind it since Big Sur put system libraries in the dyld
shared cache, plus five /usr/local/lib OpenEXR 2.2 paths from a
Homebrew layout. zlib is now linked by name through the SDK stub, and
the OpenEXR entries apply only when OPTION_BUILD_EXR is on.
Two things assumed an x86 target. The bc7e ISPC command line hardcoded --target=sse2,sse4,avx,avx2 and passed no --arch, so it inherited the ispc host default. That is correct on a Linux or Windows x86-64 host and wrong on an arm64 host, in both directions: the x86-64 slice needs an explicit --arch=x86-64, and an arm64 slice needs the NEON target instead of the four x86 ones. BC7ENC_RDO_ISPC_ARCH and BC7ENC_RDO_ISPC_TARGETS are now cache variables whose defaults reproduce the existing build exactly. The object list follows the target list, and drops the per-target objects when only one target is requested, because ispc emits the dispatch object only for multi-target builds. CMP_Core_SSE, CMP_Core_AVX and CMP_Core_AVX512 are x86 intrinsic code compiled at three instruction set levels with -march=nehalem, haswell and skylake-avx512. They cannot compile for arm64 and are now built only for x86 targets. core_simd.h defines CMP_CORE_X86_SIMD and gates the three declarations, and bc1_cmp.h gates the dispatch chain, so the scalar _cpu_bc1ComputeBestEndpoints stands alone elsewhere. That is the path every non-Windows host already takes at run time, since GetCPUExtensions reports nothing there. Target architecture comes from CMAKE_OSX_ARCHITECTURES when it is set and CMAKE_SYSTEM_PROCESSOR otherwise. On Apple platforms the former names the slice being built while the latter still reports the host, so an arm64 host cross-building the x86_64 slice needs these libraries.
Companion to tools/linux/build_flavor.sh and
tools/win/build_cli_batch.ps1: same three flavors, same flag intent,
same build-dir naming with the architecture appended. One build per
architecture; join them with lipo -create for a universal binary.
Differences from the Linux recipe, and why:
- No -static. Darwin has no static libSystem. The binary links
libSystem, libc++ and libz, all shipped with the OS, which needs no
install — the same practical result as the Windows build's 3 DLLs.
- CMAKE_OSX_ARCHITECTURES selects the slice. An arm64 host
cross-builds the x86_64 slice, which then runs under Rosetta 2.
- The bc7e ISPC target list follows the architecture.
The script refuses one combination outright: an arm64 ispc host against
a bc7e.ispc that still uses bare `--` on varying unsigned values. ispc
1.19 and later miscompile that into a no-op when the compiler itself is
an aarch64 build (ispc#3882, bc7enc_rdo#23), which corrupts bc7e's block
bit packing for every target, not only arm64. With assertions the
failure is loud — "bc7e.ispc:2890: Assertion failed: *pCur_ofs <= 128" —
but this recipe passes --opt=disable-assertions, so it would otherwise
ship textures roughly 25dB worse with no warning at all.
Either fix makes the build safe, and the check accepts both:
bc7enc_rdo#29, which rewrites the five sites as `x -= 1` and is
byte-identical to an unpatched build made with an x86_64 host; or the
macOS x86_64 ispc package.
.gitignore covers the new per-architecture build directories.
Owner
|
Nice work! |
elseform
added a commit
to elseform/atak
that referenced
this pull request
Aug 12, 2026
…uild The embedded binary crashed with a stack buffer overflow (__stack_chk_fail inside CompressBlockBC7_bc7enc_from_double_opts) on every hard-alpha texture (binary 0/255 alpha, e.g. glass textures). Root cause: ispc >=1.19 hosted on aarch64 silently miscompiles `x--` into a no-op on varying unsigned values (ispc#3882), corrupting bc7e.ispc's bit-packing — undetected because the build passes --opt=disable-assertions. Confirmed via lldb backtrace and direct reproduction; fixed by applying the patch from bc7enc_rdo#29 (x-- -> x -= 1) to the bc7enc_rdo checkout used to build this binary, and rebuilding via the (much more thorough) macOS build recipe from noisethanks/compressonator#1, which independently found and guards against the same ispc bug. Verified: all 7 previously-crashing files (hard-alpha character/glass textures) now compress successfully with a clean 4:1 BC7 ratio.
elseform
added a commit
to elseform/atak
that referenced
this pull request
Aug 12, 2026
…uild Same fix as dev (24cb03c): the embedded binary crashed with a stack buffer overflow on hard-alpha textures due to an ispc host-compiler miscompilation (ispc#3882) corrupting bc7e.ispc's bit-packing. Rebuilt using the bc7enc_rdo#29 patch and the macOS build recipe from noisethanks/compressonator#1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes §8 up on "macOS: out of scope. Contributions welcome." Adds
tools/macos/build_flavor.shand the source fixes needed to make it produce a correct binary. Built and verified on an M4 Max:Mach-O universal (x86_64 + arm64), linking onlylibSystem,libc++andlibz.Four commits, deliberately separable. The first one is not macOS-specific and is the most important.
1.
Use atomics for the BC7 worker handoff, not volatileThe producer hands work to a worker by setting
run = TRUEand reclaims the slot when it readsrun == FALSE. Both flags werevolatile CMP_BOOL, which orders nothing between threads. On a weakly ordered CPU the producer can see a slot go idle before the worker's writes to*outare visible, then reuse it while the worker is still readingin/batch_in.Encoding one 512×512 image ten times on macOS arm64, before the fix:
OPTION_CMP_USE_BC7ENC_RDO=OFF)One bc7e run lost 19 dB PSNR against the
-NumThreads 1reference. One segfaulted.-NumThreads 1was always correct, which points at the handoff rather than either codec.x86-64's store ordering hides this completely, which is why Linux and Windows never showed it. Stock BC7 fails harder than the bc7e path, so this is an upstream AMD defect that the bc7e work merely made visible — it looks worth reporting to GPUOpen-Tools independently of this branch. After the fix all tested configurations return one output across twelve runs, matching
-NumThreads 1byte for byte.2.
Fix the CLI-only build on macOSFour independent defects. The nastiest is silent: the C++ feature probe skipped Apple hosts and left them on C++11, which disables the
std::filesystembranch incmp_fileio.cpp, soCMP_GetJustFileExtreturnsddsinstead of.dds,IsDestinationUnCompressed()answers true for every destination, and the CLI writes a decompressed DDS at 4× the expected size while reporting success and exit 0. That is very likely upstream issue GPUOpen-Tools#168. The others:GetCPUIDleft its buffer uninitialized outside Windows so macOS chose BC1 SIMD kernels from stack garbage;canalysisrequired OpenCV even withOPTION_CMP_OPENCV=OFF; and the Apple link list hardcoded/usr/lib/libz.dylib, which has had no file behind it since Big Sur.3.
Build CMP_Core for non-x86 targetsBC7ENC_RDO_ISPC_ARCH/_TARGETSbecome cache variables whose defaults reproduce the current build exactly, and the three x86 SIMD sub-libraries are built only for x86 targets, with their declarations and BC1 dispatch gated onCMP_CORE_X86_SIMD. Target arch prefersCMAKE_OSX_ARCHITECTURESoverCMAKE_SYSTEM_PROCESSOR, so an arm64 host can cross-build the x86_64 slice.4.
Add tools/macos/build_flavor.shSame three flavors and flag intent as the Linux script, minus
-static(Darwin has no static libSystem).It refuses one combination outright: an arm64 ispc host against a
bc7e.ispcthat still uses bare--on varying unsigned values. ispc ≥ 1.19 miscompiles that into a no-op when the compiler itself is an aarch64 build (ispc#3882, bc7enc_rdo#23), corrupting bc7e's bit packing for every target, not only arm64. With assertions it is loud; this recipe passes--opt=disable-assertions, so it would otherwise ship textures ~25 dB worse with no warning. Either bc7enc_rdo#29 or the x86_64 ispc package makes it safe, and the check accepts both.Verification
-Quality 1.0: quality within ±0.7 dB either way, time 0.54 s vs 24.70 s for three textures (~46×). Same ordering §5 reports.Draft because the README (§4 build table, §8 platform table) still needs updating, and because the byte-identity wording in §8 is yours to decide.