Skip to content

fix(test): include integer_ssim.h unconditionally in i686 no-asm build - #700

Merged
lusoris merged 1 commit into
masterfrom
fix/i686-ssim-moments-type-test
Jun 6, 2026
Merged

lusoris merged 1 commit into
masterfrom
fix/i686-ssim-moments-type-test

Conversation

@lusoris

@lusoris lusoris commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • test_integer_ssim_simd.c uses integer_ssim_moments_t in the unconditional scalar reference functions (scalar_accumulate_row_8, scalar_accumulate_row_16) but the only include that brings in that type is gated behind #if ARCH_X86
  • On i686 no-asm builds (--cross-file=build-aux/i686-linux-gnu.ini -Denable_asm=false), ARCH_X86 is NOT defined in config.h — src/meson.build only sets it inside if is_asm_enabled
  • Result: two error: unknown type name 'integer_ssim_moments_t' compile errors; CI job "Build — Ubuntu i686 gcc (CPU, no-asm)" has failed since PR feat(simd): AVX2 integer SSIM horizontal moment accumulation #145 merged
  • Fix: add an unconditional #include "integer_ssim.h" (the shared type header introduced by ADR-1040 / PR fix(build): restore integer_ssim_moments_t type definition (macOS Clang unblock) #654) before the arch-gated AVX2 include

The AVX2 function calls remain inside #if ARCH_X86 so the linker does not need those symbols on no-asm builds.

Reproducer

meson setup build-i686 --cross-file=build-aux/i686-linux-gnu.ini -Denable_asm=false
ninja -C build-i686 test/test_integer_ssim_simd
# Before: test_integer_ssim_simd.c:102: error: unknown type name 'integer_ssim_moments_t'
# After: builds cleanly

Root cause chain

  1. PR feat(simd): AVX2 integer SSIM horizontal moment accumulation #145 — introduced integer_ssim_moments_t in x86/integer_ssim_avx2.h and the test
  2. PR fix(build): restore integer_ssim_moments_t type definition (macOS Clang unblock) #654 (ADR-1040) — promoted the typedef to integer_ssim.h for macOS arm64, fixed integer_ssim.c, but missed test_integer_ssim_simd.c
  3. This PR — fixes the test file

Deliverables checklist

  • research digest: no digest needed: trivial missing-include fix
  • decision matrix: no alternatives: only-one-way fix (add the missing include)
  • AGENTS.md invariant note: no rebase-sensitive invariants
  • reproducer: meson setup build-i686 --cross-file=build-aux/i686-linux-gnu.ini -Denable_asm=false && ninja -C build-i686 test/test_integer_ssim_simd
  • changelog fragment: see below
  • rebase-notes.md: no rebase impact: test-only single-line include fix

🤖 Generated with Claude Code

…m_simd.c

On i686 no-asm builds (enable_asm=false), ARCH_X86 is not set in
config.h — src/meson.build only defines ARCH_X86 inside the
`if is_asm_enabled` block.  The scalar reference functions
scalar_accumulate_row_8() and scalar_accumulate_row_16() use
integer_ssim_moments_t unconditionally (they are the scalar reference
that any architecture can run), but the only include that brought in the
type was guarded by #if ARCH_X86 (via feature/x86/integer_ssim_avx2.h
-> ../integer_ssim.h).  On no-asm i686, that guard is false, so the
type was never defined, giving:

  test_integer_ssim_simd.c:102: error: unknown type name 'integer_ssim_moments_t'
  test_integer_ssim_simd.c:129: error: unknown type name 'integer_ssim_moments_t'

Fix: add an unconditional #include "integer_ssim.h" before the #if
ARCH_X86 block.  The shared header was introduced by ADR-1040 / PR #654
to solve the same class of problem in integer_ssim.c for macOS arm64;
the test file was not updated at that time.  The test's -I../src/feature
include path resolves the header without a path prefix.

The AVX2 function calls in the test remain inside #if ARCH_X86, so the
linker only needs those symbols when asm is enabled, which is correct.

Fixes: Build — Ubuntu i686 gcc (CPU, no-asm) (CI run 26953952042)
Introduced by: PR #145 (feat: AVX2 integer SSIM horizontal moment accumulation)
Partial-fix-of: ADR-1040 (which fixed integer_ssim.c but missed the test)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris
lusoris marked this pull request as ready for review June 6, 2026 01:03
@lusoris
lusoris merged commit 128b4d9 into master Jun 6, 2026
72 of 107 checks passed
@lusoris
lusoris deleted the fix/i686-ssim-moments-type-test branch June 6, 2026 01:04
lusoris added a commit that referenced this pull request Jun 6, 2026
…06 batch (#719)

Refreshes bug-tracking and planning documents after the 18-PR June 5-6 batch
(#691–#711). Changes in docs/state.md (tracked):

- Add batch-sweep _Updated header summarising PRs #691–#711.
- Add T-NEON-FMA-FLOAT-ADM-DWT2-2026-06-06 to Open bugs table (was only in
  the _Updated header, missing from the table body).
- Remove duplicate T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 row (stale copy
  that cited closed PR #215 as OPEN; the de-cited version is kept).
- Fix concatenated duplicate T-GPU-COVERAGE-STABLE-WEEKS rows in Recently
  closed (two entries were joined on a single line without a separator).
- Add 8 new Recently closed rows for substantive June 5-6 fixes:
  T-NEON-FMA-FLOAT-ADM-DWT2-REVERT (PR #695 revert),
  T-METAL-FEATURE-COLLECTOR-EXTERN-C (PR #694),
  T-SYCL-DICT-INCLUDE-MISSING (PR #696),
  T-INTEGER-SSIM-I686-INCLUDE (PR #700),
  T-WAVE8-OBJ-TARGET-DEPS (PRs #699/#701),
  T-DNN-INT8-TEST-ADR1032-ALIGN (PR #705),
  T-MCP-SMOKE-11-FAILURES (PR #706).

OPEN.md and BACKLOG.md (gitignored, updated in main tree only):
- Header dates updated to 2026-06-06.
- SIMD divergence cluster marked CLOSED (PR #681, 2026-06-04).
- T-NEON-FMA-FLOAT-ADM-DWT2 added to Active right now.
- June 5-6 PR table added to Recently completed sections.
- Awaiting user decision updated: SIMD divergence closed, NEON FMA rewire new.

no rebase impact: docs/state.md only; no C sources touched.

Co-authored-by: Lusoris <lusoris@pm.me>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
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.

1 participant