feat(pass)!: rebuild pass framework on LLVM new pass manager model#74
Conversation
Replace the reflection-based pass.Manager with a generics-based design modeled on LLVM's new pass manager: - pass.Manager: lazy analysis cache (Register/GetResult/Invalidate); only reflect.TypeFor[R]() remains as a map key, no dynamic method dispatch - pass.Pipeline[U]: ordered transform sequence (AddPass/Run) - Analysis[U,R] and Pass[U] interfaces; Preserved-based invalidation - BasicBlocksPass -> BasicBlocksAnalysis - transforms return pass.Preserved and request blocks via GetResult - optimize: declarative O0/O1/O2 levels with a level->passes table - new AlgebraicSimplificationPass: integer identities + strength reduction BREAKING CHANGE: pass package API replaced; Manager.Load/Run/Convert and Optimizer.Register removed in favor of GetResult/Pipeline/AddPass.
PR Review — feat(pass)!: rebuild pass framework on LLVM new pass manager modelDecision: Request ChangesMerge Readiness SummaryThe PR successfully redesigns the pass/analysis framework with a cleaner, more type-safe generics-based approach following LLVM's new pass manager. The architecture is sound, tests are comprehensive, and documentation is well-updated. However, the build is currently failing due to incompatible function signatures in Blocking FindingsCompilation Error in interp/interp.goIssue: Build logs show compilation failures:
Why it matters: Code cannot compile, which blocks merge. Minimal fix: Update both Validation: Run Detailed Assessment✅ Scope ControlFocused on one concern: rebuilding the pass framework. No unrelated cleanups. Directly addresses architectural issue. ✅ Correctness (except CI issue)
✅ Test Coverage
✅ Architecture Consistency
✅ Risk & Compatibility
✅ Maintainability
Minimal Path to Merge
Final RecommendationFix blocking findings and re-run validation. The architectural work here is excellent and represents a significant improvement over the reflection-based approach. This is a well-executed framework redesign that will make the codebase safer and more maintainable. Once the compilation error is resolved, this should be merge-ready. Generated by Claude Code |
Commit 61b93f1 removed the cutoff int param from newCompiler and its call sites but left the !arm64 stub taking an int, breaking non-arm64 builds. Drop the unused parameter so the stub matches.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
- Coverage 64.04% 63.36% -0.69%
==========================================
Files 60 63 +3
Lines 10859 11782 +923
==========================================
+ Hits 6955 7466 +511
- Misses 3184 3526 +342
- Partials 720 790 +70 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Changes Made
Rebuild the analysis/optimization framework on LLVM's new pass manager model, removing heavy reflection and an awkward interface.
passpackagepass.Manager— lazy analysis cache:Register[U,R],GetResult[R](m, unit),Invalidate(Preserved). Only residual reflection isreflect.TypeFor[R]()as a map key; no dynamic method dispatch.pass.Pipeline[U]— ordered transform sequence:AddPass,Run(m, unit), invalidating stale analyses between passes.Analysis[U,R]andPass[U]interfaces;Preserved(PreserveAll/PreserveNone).*pass.Manageris the first argument everywhere (context-style ordering).analysis / transform / optimize
BasicBlocksPass→BasicBlocksAnalysis;Runtakes its unit directly.GetResultand returnpass.Preserved.optimizeuses a declarative level→passes table with O0/O1/O2 (O1 = CF+CD, O2 adds AlgebraicSimplification + DCE).AlgebraicSimplificationPass: integer (I32/I64) identities (x+0,x*1,x&-1, …) and strength reduction (x*2ⁿ→<<n, unsignedx/2ⁿ→>>n). Float identities skipped (IEEE-754).Docs:
pass-system.md,architecture.md,coding-patterns.mdupdated.BREAKING CHANGE:
passpublic API replaced —Manager.Load/Run/ConvertandOptimizer.Registerremoved in favor ofGetResult/Pipeline/AddPass.Related Issues
Additional Information
Test plan:
go build ./...go vet ./..., gofmt cleango test -race ./pass/... ./analysis/... ./transform/... ./optimize/...as_test.gocovers each identity/strength-reduction case;Managercache +Invalidatetest;O2optimizer case