Skip to content

Port internal self-tests out of libHalide into test/correctness - #9276

Open
alexreinking wants to merge 3 commits into
mainfrom
alexreinking/port-internal-tests-to-correctness
Open

Port internal self-tests out of libHalide into test/correctness#9276
alexreinking wants to merge 3 commits into
mainfrom
alexreinking/port-internal-tests-to-correctness

Conversation

@alexreinking

@alexreinking alexreinking commented Aug 4, 2026

Copy link
Copy Markdown
Member

Halide has historically embedded self-test functions (e.g. Foo::test(), foo_test()) directly in src/*.cpp, invoked from a single test/internal.cpp binary. This moves every one of those test bodies (and their exclusive helper functions) out of libHalide entirely, into standalone test/correctness/*.cpp executables that exercise the same internal APIs via Halide::Internal::, following the same conventions as the rest of the correctness suite.

test/internal.cpp and the _test_internal test target are removed; a trivial test/pch_helper.cpp takes over _test_internal's role as the precompiled-header donor for the other test targets.

Two internal APIs needed small additions to support this:

  • CodeGen_C.h gained three accessor functions (codegen_c_test_*) since the binary2cpp-generated blobs its test compares against aren't exported from the shared library.
  • spirv_ir.cpp gets its own CMake wiring for the internal-only SpirvIR.h header and vendored SPIR-V headers.

Breaking changes

None

Checklist

  • Tests added or updated (not required for docs, CI config, or typo fixes)
  • Commits include AI attribution where applicable (see Code of Conduct)

@alexreinking
alexreinking requested a review from abadams August 4, 2026 20:00
@alexreinking

Copy link
Copy Markdown
Member Author

Fidelity check: did the test bodies actually move unchanged?

Since each ported test's body still exists as pure deletions in the still-present src/*.cpp file, and each new test/correctness/*.cpp file is 100% new (so its content is pure additions), git diff between this commit (0cb496c08) and its parent (734f678e1) gives an exact before/after pair for every port. I stripped the mechanical porting scaffolding (#include "Halide.h", using namespace ..., the main() wrapper, printf("Success!\n")) from both sides and diffed the remainder with whitespace ignored.

Result: 94.7% of the ported content (1864 / 1968 lines) is identical, modulo whitespace.

src/*.cpp (old) test/correctness/*.cpp (new) old new equal match %
IRPrinter.cpp ir_printer.cpp 45 43 39 86.7%
CodeGen_C.cpp codegen_c.cpp 158 188 147 78.2%
IREquality.cpp ir_equality.cpp 66 33 25 37.9%
Bounds.cpp bounds_internal.cpp 413 424 410 96.7%
IRMatch.cpp expr_match.cpp 27 25 24 88.9%
Deinterleave.cpp deinterleave_vector.cpp 30 28 28 93.3%
ModulusRemainder.cpp modulus_remainder.cpp 25 23 23 92.0%
CSE.cpp cse.cpp 136 133 128 94.1%
CPlusPlusMangle.cpp cplusplus_mangle.cpp 404 403 402 99.5%
Monotonic.cpp is_monotonic.cpp 68 66 66 97.1%
Reduction.cpp split_predicate.cpp 52 52 50 96.2%
Associativity.cpp associativity.cpp 260 261 257 98.5%
Generator.cpp generator_internal.cpp 152 154 152 98.7%
AutoScheduleUtils.cpp propagate_estimate.cpp 22 20 19 86.4%
UniquifyVariableNames.cpp uniquify_variable_names.cpp 69 64 62 89.9%
SpirvIR.cpp spirv_ir.cpp 41 38 32 78.0%
TOTAL 1968 1955 1864 94.7%

I inspected the residual (non-matching) lines for every file by hand. They fall into exactly two buckets:

1. Purely mechanical. The old .cpp files had file-scope using std::vector; / using std::string; / etc., so the moved code used bare vector/string/ostringstream. The standalone test files don't carry those using declarations over, so the same lines now read std::vector/std::string/std::ostringstream. This accounts for the bulk of the "residual" diff in ir_printer.cpp, expr_match.cpp, and uniquify_variable_names.cpp.

2. Deliberate, disclosed changes (each has an explanatory comment in the new file):

  • ir_equality.cpp (37.9%, the lowest match) — the original used a private Comparer<N> template and Order enum that only existed in IREquality.cpp's own anonymous namespace, unreachable from outside that translation unit. Rewritten to use the public graph_equal/graph_less_than functions instead, preserving the same 3 test cases and the same intent (equality check, plus an antisymmetric-ordering check for the "hangs if comparison is exponential" case).
  • codegen_c.cpp (78.2%) — the 3 binary2cpp-generated blob symbols (halide_c_template_CodeGen_C_prologue, etc.) it compares generated output against turned out not to be exported from the shared library (Halide's linker export script only allows halide_* runtime-API symbols and mangled Halide:: C++ symbols — not raw data blobs). Added 3 small Halide::Internal::codegen_c_test_* accessor functions to CodeGen_C.h/.cpp and call those instead.
  • spirv_ir.cpp (78.0%) — dropped the dead #ifdef WITH_SPIRV / "SpirV IR test disabled" fallback branch, since the new test target always defines WITH_SPIRV; also swapped bare assert() for internal_assert() to match Halide's convention.
  • bounds_internal.cpp (96.7%, only mentioning for the one real addition) — reimplemented a small private Interval simplify(const Interval&) overload that lived in Bounds.cpp's own anonymous namespace, since it isn't part of the public API surface.

Everything else — Deinterleave.cpp, ModulusRemainder.cpp, CSE.cpp, CPlusPlusMangle.cpp, Monotonic.cpp, Reduction.cpp, Associativity.cpp, Generator.cpp, AutoScheduleUtils.cpp — moved with zero behavioral change, only the mechanical main()/printf("Success!\n") wrapping.

Script and commands used to produce this (click to expand)

Run from the root of a checkout that has both commits in its history:

python3 compare_ports.py          # summary table (shown above)
python3 compare_ports.py --full   # + full unified diff per file, for spot-checking

compare_ports.py:

#!/usr/bin/env python3
"""Correlate deleted src/*.cpp test bodies with their new test/correctness/*.cpp
homes, and report how much of the diff is "real" content change vs mechanical
renaming/whitespace, by diffing the two blocks of text with whitespace ignored.

Run from the root of a Halide checkout that has both OLD_REF and NEW_REF in
its history (e.g. after `git fetch origin pull/9276/head`):

    python3 compare_ports.py          # summary table
    python3 compare_ports.py --full   # + full unified diff per file
"""
import difflib
import re
import subprocess
import sys

OLD_REF = "734f678e1"  # parent of the porting commit
NEW_REF = "0cb496c08"  # the porting commit itself

PAIRS = [
    ("src/IRPrinter.cpp", "test/correctness/ir_printer.cpp"),
    ("src/CodeGen_C.cpp", "test/correctness/codegen_c.cpp"),
    ("src/IREquality.cpp", "test/correctness/ir_equality.cpp"),
    ("src/Bounds.cpp", "test/correctness/bounds_internal.cpp"),
    ("src/IRMatch.cpp", "test/correctness/expr_match.cpp"),
    ("src/Deinterleave.cpp", "test/correctness/deinterleave_vector.cpp"),
    ("src/ModulusRemainder.cpp", "test/correctness/modulus_remainder.cpp"),
    ("src/CSE.cpp", "test/correctness/cse.cpp"),
    ("src/CPlusPlusMangle.cpp", "test/correctness/cplusplus_mangle.cpp"),
    ("src/Monotonic.cpp", "test/correctness/is_monotonic.cpp"),
    ("src/Reduction.cpp", "test/correctness/split_predicate.cpp"),
    ("src/Associativity.cpp", "test/correctness/associativity.cpp"),
    ("src/Generator.cpp", "test/correctness/generator_internal.cpp"),
    ("src/AutoScheduleUtils.cpp", "test/correctness/propagate_estimate.cpp"),
    ("src/UniquifyVariableNames.cpp", "test/correctness/uniquify_variable_names.cpp"),
    ("src/SpirvIR.cpp", "test/correctness/spirv_ir.cpp"),
]


def git_diff_lines(path, sign):
    """Return the lines added ('+') or removed ('-') for `path` between
    OLD_REF and NEW_REF, stripped of the diff marker, in original order."""
    out = subprocess.run(
        ["git", "diff", "-U0", "--no-color", OLD_REF, NEW_REF, "--", path],
        capture_output=True, text=True, check=True
    ).stdout
    lines = []
    for line in out.splitlines():
        if line.startswith("@@") or line.startswith("diff ") or line.startswith("index "):
            continue
        if line.startswith("--- ") or line.startswith("+++ "):
            continue
        if sign == "-" and line.startswith("-"):
            lines.append(line[1:])
        elif sign == "+" and line.startswith("+"):
            lines.append(line[1:])
    return lines


# Lines that are pure "porting scaffolding" and shouldn't count against fidelity.
BOILERPLATE = re.compile(
    r'^\s*('
    r'#include "Halide\.h"|'
    r'using namespace Halide;|'
    r'using namespace Halide::Internal;|'
    r'namespace Halide \{|'
    r'namespace Internal \{|'
    r'\}\s*//\s*namespace Internal|'
    r'\}\s*//\s*namespace Halide|'
    r'int main\(.*\)\s*\{|'
    r'printf\("Success!\\n"\);|'
    r'return 0;|'
    r'\}'
    r')\s*$'
)


def strip_boilerplate(lines):
    return [l for l in lines if not BOILERPLATE.match(l.strip())]


grand_old = grand_new = grand_equal = 0

for old_file, new_file in PAIRS:
    old_lines = strip_boilerplate(git_diff_lines(old_file, "-"))
    new_lines = strip_boilerplate(git_diff_lines(new_file, "+"))

    old_norm = [l.strip() for l in old_lines if l.strip()]
    new_norm = [l.strip() for l in new_lines if l.strip()]

    sm = difflib.SequenceMatcher(a=old_norm, b=new_norm, autojunk=False)
    equal = sum(block.size for block in sm.get_matching_blocks())
    total = max(len(old_norm), len(new_norm))
    pct = 100.0 * equal / total if total else 100.0

    grand_old += len(old_norm)
    grand_new += len(new_norm)
    grand_equal += equal

    print(f"{old_file:32s} -> {new_file:48s} "
          f"old={len(old_norm):4d} new={len(new_norm):4d} equal={equal:4d} ({pct:5.1f}%)")

    if "--full" in sys.argv:
        diff = difflib.unified_diff(old_norm, new_norm,
                                     fromfile=old_file, tofile=new_file, lineterm="")
        for line in diff:
            print("   ", line)

total = max(grand_old, grand_new)
pct = 100.0 * grand_equal / total if total else 100.0
print(f"\nTOTAL: old={grand_old} new={grand_new} equal={grand_equal} ({pct:.1f}% content-identical)")

alexreinking and others added 2 commits August 4, 2026 20:58
Halide has historically embedded self-test functions (e.g. Foo::test(),
foo_test()) directly in src/*.cpp, invoked from a single test/internal.cpp
binary. This moves every one of those test bodies (and their exclusive
helper functions) out of libHalide entirely, into standalone
test/correctness/*.cpp executables that exercise the same internal APIs
via Halide::Internal::, following the same conventions as the rest of the
correctness suite.

test/internal.cpp and the _test_internal test target are removed; a
trivial test/pch_helper.cpp takes over _test_internal's role as the
precompiled-header donor for the other test targets.

Two internal APIs needed small additions to support this:
- CodeGen_C.h gained three accessor functions (codegen_c_test_*) since the
  binary2cpp-generated blobs its test compares against aren't exported
  from the shared library.
- spirv_ir.cpp gets its own CMake wiring for the internal-only SpirvIR.h
  header and vendored SPIR-V headers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_test_internal no longer runs through add_halide_test(), so it stopped
getting the same warning flags as the test targets that reuse its
precompiled header via REUSE_FROM. MSVC treats a warning-level mismatch
between a PCH and its consumer as an error under /WX (C4652).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@alexreinking
alexreinking force-pushed the alexreinking/port-internal-tests-to-correctness branch from 0357687 to 5da585e Compare August 5, 2026 00:59
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.91%. Comparing base (ceea694) to head (5c87484).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9276      +/-   ##
==========================================
- Coverage   70.26%   69.91%   -0.35%     
==========================================
  Files         257      257              
  Lines       79106    77251    -1855     
  Branches    18954    18784     -170     
==========================================
- Hits        55583    54011    -1572     
+ Misses      17885    17676     -209     
+ Partials     5638     5564      -74     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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