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 0d7c183..b708ddb 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 38cff61..a9555d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,6 +161,16 @@ 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) + +- #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 c9b9482..cb05b77 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/backend -Isrc/amdgpu -Isrc/tensix -Isrc/nvidia -Isrc/metal -Isrc/intel -Isrc/triton -Isrc/cpu -Isrc/runtime + -Isrc -Isrc/fe -Isrc/ir -Isrc/tdf -Isrc/backend -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 @@ -83,7 +88,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/backend -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 \ @@ -96,6 +101,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 \ @@ -190,12 +196,45 @@ 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) + 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 install uninstall +.PHONY: all clean test install uninstall coverage 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)