test: replace cl out.ll files with LIT checks - #2281
Conversation
There was a problem hiding this comment.
Review summary
This is a clean, well-executed migration from golden out.ll snapshots to source-embedded // LITTEST + // CHECK FileCheck directives. Dead code removal is thorough (no dangling references to SmartDoFile, DoFile, genZip, inCompilerDir, cltest.Pkg, decodeLinkFile, or the Mode enum remain), the internal/littest simplification and new ErrSpecNotFound sentinel are genuine improvements, and the removed TestCgofullGeneratesC2func is now covered by CHECK-DAG in cgofull.go. Moving defer_ir_regression_test.go off a go run ./chore/llgen subprocess to an in-process llgen.GenFrom call is also a correctness/speed win. internal/littest vets clean.
The findings below are all in files the PR did not touch, so they cannot be placed inline.
Findings
1. (High) Stale documentation references the deleted chore/gentests / chore/llgen tooling and the removed out.ll workflow. This PR deletes both chore/gentests/ and chore/llgen/ entirely and removes all out.ll files, but the docs still instruct contributors to use them — these commands will now fail and the described workflow no longer exists:
CLAUDE.md:60-102— the "Understanding out.ll files" / "Required steps after modifying IR generation logic" section still describesout.llas "generated byllgen" and instructsgo install -v ./chore/gentests,go install -v ./chore/llgen, and runninggentests/llgen.CLAUDE.md:16also listsllgenas achore/tool.README.md:425-426— the tools list still documents[llgen](chore/llgen)and[gentests](chore/gentests)(now broken in-repo links) without.ll/expect.txtregeneration descriptions.dev/README.md:77-97— the "Refresh test goldens" section documents a### gentestsflow withgo run ./chore/gentestsfor regeneratingout.ll.
Suggest updating these to describe the // LITTEST + // CHECK FileCheck workflow (and the surviving litgen tool, go run ./chore/litgen <path>) before merge. Note the internal/llgen library package still exists and is fine — only the chore/llgen command was removed.
2. (Minor) cltest.CaptureMeta is now dead exported code. cl/cltest/cltest.go:370 — CaptureMeta was only ever called from the deleted chore/gentests/gentests.go; it now has zero callers (its sibling RunAndCapture is still used by cl/compile_test.go:449). Consider removing it (and any orphaned meta-capture plumbing), or adding a comment noting an intended future caller.
3. (Minor / optional, pre-existing) IR checks now shell out per test package. With the ModeSkip short-circuit and the in-process ModeLiteral string compare both removed, every // LITTEST package now routes through filecheck.Match → llvm.New("") (internal/filecheck/filecheck.go:36), which re-execs llvm-config --bindir and then FileCheck on every call. Across the ~200 marked packages that is a meaningful number of redundant llvm-config subprocess spawns per run. This is pre-existing behavior in internal/filecheck (not introduced here), but this PR increases the number of packages routed through it. A process-level sync.Once around the llvm.New("") bindir lookup would remove the redundant execs at zero correctness cost. Not blocking.
No security concerns: removing genZip (a zip shell-out, previously dead behind if false) and decodeLinkFile (an archive/zip decode of committed fixtures) is a net reduction in surface area.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Thanks for the review. The review was generated against the initial broader version of this PR. Commit d9b118f has since narrowed the final diff to cl/_test*/** only.
The current PR diff contains only fixture sources and out.ll deletions; no tooling or test-framework code remains changed. |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
|
Expanded the migrated checks in Each of the 25 cases now checks its complete relevant function bodies and control-flow blocks instead of only a few symbols. Platform-specific matching is limited to observed ABI differences: All affected cases passed on macOS arm64 and Linux amd64. No |
Summary
out.llcases with source-embedded LIT checks covering their complete relevant functions and control flowout.llfiles: six empty files already covered by LIT checks and one unused nested artifactexpect.txtruntime-output comparisons and leavellgen,gentests, and the test framework unchangedPortability
CHECK-LABEL,CHECK-NEXT, andCHECK-EMPTYnest/swiftself, cgo hashes and C types, libc symbols, pthread layout, and reflect method slotsalloca i8without fixing its platform-defined size!dbg,DICompileUnit, andDILocalVariablemetadataCompanion documentation update: #2279
Validation
dev/docker.shgo test ./internal/littest ./internal/llgen ./chore/litgen ./chore/llgen ./chore/gentests -count=1git diff --check