From 53436e0d1adf4f7dd30288f2127d819ea4df5176 Mon Sep 17 00:00:00 2001 From: Zaneham Date: Sun, 2 Aug 2026 16:35:05 +1200 Subject: [PATCH 1/3] build: add a coverage target and CI job make coverage builds kath and trunner instrumented into their own object tree, runs the suite, and reports via gcovr. kath is instrumented too since trv_elf, ttdf and ttriton reach the backend by shelling out to it. Needs -U_FORTIFY_SOURCE (glibc warns at -O0) and -w (-O0 loses the range info -Wformat-truncation needs, so the strict set fires on fine code). The normal build stays the warnings gate. CI reports only, no threshold, until the baseline settles. --- .github/workflows/ci.yml | 26 +++++++++++++++++++++++ .gitignore | 7 ++++++ CHANGELOG.md | 4 ++++ Makefile | 46 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89f15cb..3c41988 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,32 @@ jobs: - name: Test run: make test + # Reports only, no threshold. We don't know the baseline yet, and a gate set + # from a guess just gets ignored. Add --fail-under-line once the number settles. + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install gcovr + run: sudo apt-get update && sudo apt-get install -y gcovr + - name: Build instrumented and run suite + run: make coverage + - name: Summary + if: always() + run: | + { + echo '## Coverage' + echo '```' + cat coverage.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + - name: Upload HTML report + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage-html + path: coverage-html/ + windows: runs-on: windows-latest defaults: diff --git a/.gitignore b/.gitignore index 8b909eb..09a89fb 100644 --- a/.gitignore +++ b/.gitignore @@ -149,6 +149,13 @@ CLAUDE.md /a_compute.bin /a_compute.ttinsn +# gcov instrumentation and the reports make coverage builds from it +*.gcda +*.gcno +*.gcov +/coverage.txt +/coverage-html/ + # Header dependency files from -MMD *.d tdf_flag_out.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 63ee677..8fef221 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,10 @@ Booth — Changelog - guard the divergent-return lowering against regressing to `s_endpgm` (Zane Hambly, 2026-07-26) +- #154: `make coverage` builds an instrumented tree and reports line coverage + via gcovr, with a CI job that posts the summary and uploads the HTML + (Zane Hambly, 2026-08-02) + ### Documentation - drop the LLVM requirement from the usage documentation diff --git a/Makefile b/Makefile index 83b0528..7c6a9e5 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,10 @@ else GCC_ONLY = endif +# Empty for a normal build. `make coverage` re-invokes make with this set, and +# it lands at the end of CFLAGS/TCFLAGS so its -O0 beats the -O2 above. +COVFLAGS = + CFLAGS = -std=c99 -MMD -MP -Wall -Wextra -pedantic -O2 \ -Wshadow -Wstrict-prototypes -Wmissing-prototypes \ -Wformat=2 -Wundef -Wcast-align -Wnull-dereference \ @@ -19,7 +23,8 @@ CFLAGS = -std=c99 -MMD -MP -Wall -Wextra -pedantic -O2 \ -Wdouble-promotion -Wswitch-enum -Wwrite-strings \ -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fPIE $(CF_PROT) \ $(GCC_ONLY) \ - -Isrc -Isrc/fe -Isrc/ir -Isrc/tdf -Isrc/amdgpu -Isrc/tensix -Isrc/nvidia -Isrc/metal -Isrc/intel -Isrc/triton -Isrc/cpu -Isrc/runtime + -Isrc -Isrc/fe -Isrc/ir -Isrc/tdf -Isrc/amdgpu -Isrc/tensix -Isrc/nvidia -Isrc/metal -Isrc/intel -Isrc/triton -Isrc/cpu -Isrc/runtime \ + $(COVFLAGS) LDFLAGS = -pie LIBS = -lm # Linux/ELF only: -Wl,-z,relro,-z,now -Wl,-z,noexecstack @@ -82,7 +87,7 @@ $(OBJDIR)/%.o: %.c # ---- Test Suite ---- TCFLAGS = -std=c99 -MMD -MP -D_POSIX_C_SOURCE=200809L -Wall -Wextra -O0 -g \ -Isrc -Isrc/fe -Isrc/ir -Isrc/tdf -Isrc/amdgpu -Isrc/tensix -Isrc/nvidia -Isrc/metal -Isrc/intel -Isrc/triton -Isrc/cpu -Isrc/runtime \ - -Iruntime + -Iruntime $(COVFLAGS) TSRC = tests/tmain.c tests/tsmoke.c tests/tcomp.c tests/tenc.c \ tests/ttabs.c tests/ttypes.c tests/terrs.c tests/tphase.c \ tests/tdce.c \ @@ -140,12 +145,45 @@ $(OBJDIR)/runtime/%.o: runtime/%.c @mkdir -p $(dir $@) $(CC) $(TCFLAGS) -c $< -o $@ +# ---- Coverage ---- +# Instrumented objects live in their own tree. Sharing one with the normal build +# means a later `make test` relinks against gcov objects and dies on missing +# __gcov symbols, which reads like a broken toolchain rather than a stale tree. +COVDIR := build/cov-$(UNAME_S) + +# kath is instrumented too, not just trunner: trv_elf, ttdf and ttriton shell out +# to ./kath, so a lot of the backend is only reached through the real binary. +# -U_FORTIFY_SOURCE because glibc #warnings at -O0 when it's set, and -Werror +# turns that into a build failure. +# -w because -O0 drops the value-range info that lets -Wformat-truncation prove +# its bounds at -O2, so the strict set fires on code that is fine. The normal +# build is the warnings gate; this one only counts lines. +COV_CF = --coverage -O0 -U_FORTIFY_SOURCE -w + +# Both binaries land in the repo root whichever tree built them, so clear them +# first to force an instrumented link, and again at the end so the next plain +# `make` doesn't quietly keep running the instrumented one. +coverage: + rm -f $(TARGET) $(TARGET).exe trunner trunner.exe + find $(COVDIR) -name '*.gcda' -delete 2>/dev/null || true + $(MAKE) OBJDIR=$(COVDIR) COVFLAGS="$(COV_CF)" $(TARGET) trunner + -./trunner --all + @command -v gcovr >/dev/null 2>&1 || { echo "gcovr not found. pip install gcovr"; exit 1; } + @mkdir -p coverage-html + gcovr --root . --object-directory $(COVDIR) \ + --filter 'src/' --filter 'runtime/' \ + --exclude-unreachable-branches \ + --print-summary --txt coverage.txt --html-details coverage-html/index.html + rm -f $(TARGET) $(TARGET).exe trunner trunner.exe + @echo "report: coverage.txt and coverage-html/index.html" + clean: - rm -rf $(OBJDIR) + rm -rf $(OBJDIR) $(COVDIR) rm -f $(TARGET) $(TARGET).exe trunner trunner.exe + rm -rf coverage.txt coverage-html # Header deps from -MMD. Without these a header edit leaves stale objects # linked in and the build silently disagrees with the source. -include $(OBJECTS:.o=.d) $(TOBJS:.o=.d) $(HOSTRT:.o=.d) -.PHONY: all clean test +.PHONY: all clean test coverage From 8d673737ce74f48f2718f63b999727187635a2cd Mon Sep 17 00:00:00 2001 From: Zaneham Date: Sun, 2 Aug 2026 18:15:33 +1200 Subject: [PATCH 2/3] tests: cover the SSA register allocator ra_ssa.c was at 0% -- it sits behind --ssa-ra and nothing passed the flag. These drive it end to end and check what an allocator must not get wrong: no virtual registers left behind, and nothing used past the count the kernel descriptor declares. 0% to 93%, and the tree goes 70.7% to 73.7%. Two real bugs fell out. Six fixtures (canonical, notgpt, stress, cuda_features, device_calls, test_struct) leave vregs unallocated, and so does any --max-vgprs below 8 on kernels the default allocator handles down to 2. Smallest repro is device_calls.cu, two leaks on the final store's address pair. Both are pinned as expected failures rather than skipped, so fixing the allocator trips this file and forces the lists to be updated. Also adds ra_ssa to cat_order so --cat reaches it. Unlisted categories only run in the orphan pass, unlabelled and unfilterable. --- CHANGELOG.md | 6 + Makefile | 1 + tests/tmain.c | 2 +- tests/tra_ssa.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 tests/tra_ssa.c diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fef221..73419b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,12 @@ Booth — Changelog via gcovr, with a CI job that posts the summary and uploads the HTML (Zane Hambly, 2026-08-02) +- #154: cover the SSA register allocator, which had never been run by a test. + Six fixtures and any `--max-vgprs` below 8 leave virtual registers + unallocated under `--ssa-ra`; those are pinned in `tests/tra_ssa.c` until + the allocator is fixed + (Zane Hambly, 2026-08-02) + ### Documentation - drop the LLVM requirement from the usage documentation diff --git a/Makefile b/Makefile index 7c6a9e5..5f0334d 100644 --- a/Makefile +++ b/Makefile @@ -100,6 +100,7 @@ TSRC = tests/tmain.c tests/tsmoke.c tests/tcomp.c tests/tenc.c \ tests/twarpsize.c \ tests/tabend.c \ tests/tregalloc.c \ + tests/tra_ssa.c \ tests/tguard.c \ tests/ttriton.c \ tests/ttdf.c \ diff --git a/tests/tmain.c b/tests/tmain.c index d275882..c6a9d3f 100644 --- a/tests/tmain.c +++ b/tests/tmain.c @@ -52,7 +52,7 @@ int th_exist(const char *path) static const char *cat_order[] = { "smoke", "compile", "encode", "tables", - "types", "errors", "phase", "sched", "abend", "regalloc", NULL + "types", "errors", "phase", "sched", "abend", "regalloc", "ra_ssa", NULL }; static int cat_idx(const char *cat) diff --git a/tests/tra_ssa.c b/tests/tra_ssa.c new file mode 100644 index 0000000..6a339c1 --- /dev/null +++ b/tests/tra_ssa.c @@ -0,0 +1,283 @@ +/* tra_ssa.c -- divergence-aware SSA register allocator (--ssa-ra) + * + * ra_ssa.c had no coverage at all before this file: the allocator is gated + * behind --ssa-ra and nothing passed the flag. These drive it end to end and + * check the two things an allocator must never get wrong -- leaving virtual + * registers behind, and using more registers than the kernel descriptor + * declares. A kernel that reads past its declared count reads whatever the + * previous wave left there. + * + * Six fixtures still leak vregs (see ssa_ra_rejects_cleanly). They are pinned + * here rather than skipped, so fixing the allocator fails this file and makes + * whoever fixes it move the fixture into the clean list. */ + +#include "tharns.h" + +#define SR_BUFSZ (1 << 19) /* 512KB, canonical.cu emits a lot */ + +static char sr_buf[SR_BUFSZ]; + +/* Highest register index referenced on a line, for 'v' or 's'. + * Handles both v1 and the v[0:1] pair form. Returns -1 for none. */ +static int line_max_reg(const char *ln, char pfx) +{ + int max = -1; + const char *p = ln; + + while (*p) { + /* a register only starts where the previous char isn't identifier-ish, + * otherwise s_waitcnt and lgkmcnt0 both look like registers */ + int boundary = (p == ln) || + !((p[-1] >= 'a' && p[-1] <= 'z') || + (p[-1] >= 'A' && p[-1] <= 'Z') || + (p[-1] >= '0' && p[-1] <= '9') || p[-1] == '_'); + if (*p == pfx && boundary) { + if (p[1] == '[') { /* v[lo:hi] -- hi is what matters */ + const char *c = strchr(p, ':'); + if (c && c[1] >= '0' && c[1] <= '9') { + int hi = atoi(c + 1); + if (hi > max) max = hi; + } + } else if (p[1] >= '0' && p[1] <= '9') { + int n = atoi(p + 1); + if (n > max) max = n; + } + } + p++; + } + return max; +} + +/* Walk the asm and check every kernel's declared counts actually bound the + * registers its body touches. Returns 0 if clean, -1 on an overrun. */ +static int check_bounds(const char *buf) +{ + const char *p = buf; + int dv = -1, ds = -1, maxv = -1, maxs = -1; + char ln[1024]; + + while (*p) { + const char *nl = strchr(p, '\n'); + size_t len = nl ? (size_t)(nl - p) : strlen(p); + if (len >= sizeof ln) len = sizeof ln - 1; + memcpy(ln, p, len); + ln[len] = '\0'; + + /* "; 14 SGPRs, 5 VGPRs, ..." opens a new function */ + const char *sg = strstr(ln, " SGPRs"); + if (sg) { + int a = 0, b = 0; + if (sscanf(ln, "; %d SGPRs, %d VGPRs", &a, &b) == 2) { + ds = a; dv = b; maxv = -1; maxs = -1; + } + } else { + int v = line_max_reg(ln, 'v'); + int s = line_max_reg(ln, 's'); + if (v > maxv) maxv = v; + if (s > maxs) maxs = s; + } + + if (strstr(ln, "s_endpgm") && dv >= 0) { + if (maxv >= dv) { + printf(" VGPR overrun: declared %d, used v%d\n", dv, maxv); + return -1; + } + if (maxs >= ds) { + printf(" SGPR overrun: declared %d, used s%d\n", ds, maxs); + return -1; + } + dv = -1; ds = -1; + } + + if (!nl) break; + p = nl + 1; + } + return 0; +} + +/* Compile with --ssa-ra. Returns kath's exit code, output in sr_buf. */ +static int ssa_compile(const char *cu, const char *extra) +{ + char cmd[TH_BUFSZ]; + snprintf(cmd, TH_BUFSZ, BC_BIN " --amdgpu --ssa-ra %s %s", extra, cu); + return th_run(cmd, sr_buf, SR_BUFSZ); +} + +/* Fixtures the SSA allocator handles today. */ +static const char *sr_clean[] = { + "tests/vector_add.cu", + "tests/tiny.cu", + "tests/test_vadd.cu", + "tests/test_loop.cu", + "tests/test_branch.cu", + "tests/test_cf.cu", + NULL +}; + +/* Fixtures where it still leaves vregs unallocated. */ +static const char *sr_leaky[] = { + "tests/canonical.cu", + "tests/notgpt.cu", + "tests/stress.cu", + "tests/cuda_features.cu", + "tests/device_calls.cu", + "tests/test_struct.cu", + NULL +}; + +/* ---- The allocator finishes its job ---- */ + +/* Every vreg must be gone by the time RA is done. verify.c says so too, but + * asserting it here names the actual failure instead of an exit code. */ +static void ssa_ra_no_vreg_leak(void) +{ + int i; + for (i = 0; sr_clean[i]; i++) { + int rc = ssa_compile(sr_clean[i], ""); + if (rc != 0) { + printf(" %s: exit %d\n %s\n", sr_clean[i], rc, sr_buf); + CHECK(0); + } + if (strstr(sr_buf, "still present after RA")) { + printf(" %s: allocator left virtual regs behind\n", sr_clean[i]); + CHECK(0); + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_no_vreg_leak) + +/* ---- Declared registers bound the ones actually used ---- */ + +static void ssa_ra_within_declared(void) +{ + int i; + for (i = 0; sr_clean[i]; i++) { + CHEQ(ssa_compile(sr_clean[i], ""), 0); + if (check_bounds(sr_buf) != 0) { + printf(" in %s\n", sr_clean[i]); + CHECK(0); + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_within_declared) + +/* Same check with the budget squeezed enough to force spilling. */ +static void ssa_ra_within_declared_spilling(void) +{ + int i; + for (i = 0; sr_clean[i]; i++) { + int rc = ssa_compile(sr_clean[i], "--max-vgprs 8"); + if (rc != 0) { + printf(" %s: exit %d\n", sr_clean[i], rc); + CHECK(0); + } + CHECK(strstr(sr_buf, "s_endpgm") != NULL); + if (check_bounds(sr_buf) != 0) { + printf(" in %s\n", sr_clean[i]); + CHECK(0); + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_within_declared_spilling) + +/* Squeeze harder and the spill path leaks vregs too, on kernels the default + * allocator handles down to --max-vgprs 2. Same deal as sr_leaky: pinned so a + * fix trips this and gets folded into the test above. */ +static void ssa_ra_tight_cap_rejects_cleanly(void) +{ + const char *caps[] = { "--max-vgprs 4", "--max-vgprs 2", NULL }; + int c; + + for (c = 0; caps[c]; c++) { + int rc = ssa_compile("tests/vector_add.cu", caps[c]); + if (rc == 0) { + printf(" vector_add now survives %s, fold it back in\n", caps[c]); + CHECK(0); + } + if (!strstr(sr_buf, "still present after RA")) { + printf(" %s: failed but not on a vreg leak\n", caps[c]); + CHECK(0); + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_tight_cap_rejects_cleanly) + +/* ---- Known-broken fixtures fail loudly ---- */ + +/* These leak vregs. What matters until that is fixed is that verify catches + * it and we exit non-zero, rather than quietly emitting a broken kernel. + * Fix the allocator and this test fails: move the fixture to sr_clean. */ +static void ssa_ra_rejects_cleanly(void) +{ + int i; + for (i = 0; sr_leaky[i]; i++) { + int rc = ssa_compile(sr_leaky[i], ""); + if (rc == 0) { + printf(" %s now passes --ssa-ra, move it to sr_clean\n", sr_leaky[i]); + CHECK(0); + } + if (!strstr(sr_buf, "still present after RA")) { + printf(" %s: failed but not on a vreg leak\n", sr_leaky[i]); + CHECK(0); + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_rejects_cleanly) + +/* ---- The allocator doesn't take the compiler down ---- */ + +/* Bad output is one thing, a crash is another. Everything above runs through + * here with the budget squeezed hard, and a clean rejection is fine. */ +static void ssa_ra_survives_pressure(void) +{ + const char *caps[] = { "", "--max-vgprs 4", "--max-vgprs 2", NULL }; + int c, i; + + for (c = 0; caps[c]; c++) { + for (i = 0; sr_leaky[i]; i++) { + int rc = ssa_compile(sr_leaky[i], caps[c]); + if (rc != 0 && rc != 1) { + printf(" %s %s: exit %d, not a clean rejection\n", + sr_leaky[i], caps[c], rc); + CHECK(0); + } + } + for (i = 0; sr_clean[i]; i++) { + int rc = ssa_compile(sr_clean[i], caps[c]); + if (rc != 0 && rc != 1) { + printf(" %s %s: exit %d, not a clean rejection\n", + sr_clean[i], caps[c], rc); + CHECK(0); + } + } + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_survives_pressure) + +/* ---- Both allocators agree on the shape of what they emit ---- */ + +/* Not a text diff: the whole point is that they allocate differently. But a + * kernel is still a kernel, so the entry label and terminator must survive. */ +static void ssa_ra_emits_a_kernel(void) +{ + char cmd[TH_BUFSZ]; + int i; + + for (i = 0; sr_clean[i]; i++) { + CHEQ(ssa_compile(sr_clean[i], ""), 0); + CHECK(strstr(sr_buf, "s_endpgm") != NULL); + CHECK(strstr(sr_buf, ".amdgcn_target") != NULL); + + snprintf(cmd, TH_BUFSZ, BC_BIN " --amdgpu %s", sr_clean[i]); + CHEQ(th_run(cmd, sr_buf, SR_BUFSZ), 0); + CHECK(strstr(sr_buf, "s_endpgm") != NULL); + } + PASS(); +} +TH_REG("ra_ssa", ssa_ra_emits_a_kernel) From 6640c17bc09407bf2e58ecba0b4a5a2728651372 Mon Sep 17 00:00:00 2001 From: ZaneHam Date: Fri, 7 Aug 2026 00:38:26 +1200 Subject: [PATCH 3/3] build: restore the coverage target lost in the master merge --- Makefile | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8fb67b8..ac342e2 100644 --- a/Makefile +++ b/Makefile @@ -185,6 +185,38 @@ uninstall: rm -f $(BINDIR)/$(TARGET)$(EXE) rm -rf $(SHAREDIR) $(CMAKEDIR) +# ---- Coverage ---- +# Instrumented objects live in their own tree. Sharing one with the normal build +# means a later `make test` relinks against gcov objects and dies on missing +# __gcov symbols, which reads like a broken toolchain rather than a stale tree. +COVDIR := build/cov-$(UNAME_S) + +# kath is instrumented too, not just trunner: trv_elf, ttdf and ttriton shell out +# to ./kath, so a lot of the backend is only reached through the real binary. +# -U_FORTIFY_SOURCE because glibc #warnings at -O0 when it's set, and -Werror +# turns that into a build failure. +# -w because -O0 drops the value-range info that lets -Wformat-truncation prove +# its bounds at -O2, so the strict set fires on code that is fine. The normal +# build is the warnings gate; this one only counts lines. +COV_CF = --coverage -O0 -U_FORTIFY_SOURCE -w + +# Both binaries land in the repo root whichever tree built them, so clear them +# first to force an instrumented link, and again at the end so the next plain +# `make` doesn't quietly keep running the instrumented one. +coverage: + rm -f $(TARGET) $(TARGET).exe trunner trunner.exe + find $(COVDIR) -name '*.gcda' -delete 2>/dev/null || true + $(MAKE) OBJDIR=$(COVDIR) COVFLAGS="$(COV_CF)" $(TARGET) trunner + -./trunner --all + @command -v gcovr >/dev/null 2>&1 || { echo "gcovr not found. pip install gcovr"; exit 1; } + @mkdir -p coverage-html + gcovr --root . --object-directory $(COVDIR) \ + --filter 'src/' --filter 'runtime/' \ + --exclude-unreachable-branches \ + --print-summary --txt coverage.txt --html-details coverage-html/index.html + rm -f $(TARGET) $(TARGET).exe trunner trunner.exe + @echo "report: coverage.txt and coverage-html/index.html" + clean: rm -rf $(OBJDIR) $(COVDIR) rm -f $(TARGET) $(TARGET).exe trunner trunner.exe @@ -194,4 +226,4 @@ clean: # linked in and the build silently disagrees with the source. -include $(OBJECTS:.o=.d) $(TOBJS:.o=.d) $(HOSTRT:.o=.d) -.PHONY: all clean test install uninstall +.PHONY: all clean test install uninstall coverage