Repository navigation
perf(ir): fold constant branches before the first typeinfer epoch - #1102
Conversation
| (#'ir.passes.pipeline/run-pass-list | ||
| f (mapv wrap ir.passes.pipeline/default-pass-list)) | ||
| @late))`) | ||
| if got != "0" { |
There was a problem hiding this comment.
I just realized that this test method returns a string, and not a vm.Int or vm.Value
There was a problem hiding this comment.
Good catch. The epoch test below does the same thing, so I switched both to compare values in #1103, stacked on this one.
a32704c to
8733bba
Compare
nnunley
left a comment
There was a problem hiding this comment.
Reviewed current head 8733bba. Source inspection and focused type-inference/scheduling tests found no confirmed optimizer correctness issue. The string-helper assertions currently use pr-str consistently; #1103 improves both tests with direct value comparisons, and I have approved that follow-up.
The remaining concern is validation before merge:
- Build attempt 3 exhausts the short e2e package's 60-second budget while
TestTermFlushWithPipedStdoutis still insidebuildLGwaiting forgo build. The term script has not started. Subsequent compatibility, bootstrap, expensive-lowering, and native-entry steps are skipped. - The exact base's build passed, including the short e2e suites. Attribution remains unresolved. The inherited per-test build helper is relevant, but the evidence does not establish a failing base or justify calling this flaky.
- #1104 directly addresses repeated binary builds and is now approved. Landing it and updating this branch is a reasonable next step; please obtain a passing build on the resulting delivered revision, including the later gates currently skipped. If the timeout persists, compare the full short-e2e package on base/head under matched Linux conditions.
I would hold merging until that current-revision build is green. No optimizer code change is requested from the timeout evidence alone.
Since #1087, branchfold made lowering core about 15% slower. Two costs: - known-truth ran arg-derived? on every branch-if test before reading the test's type. That trace follows block params back through their predecessors and resolves callee vars, and its answer only matters when the type would fold. It now runs only then; the results are the same. - Every fold in core is on a constant test, like the (if :else x nil) a macro can leave, yet each one waited until after the first epoch and then cost a second, from-scratch epoch. branchfold-constants folds constant tests before the first epoch, which then infers the folded CFG once. branchfold-1 and branchfold-2 still fold tests that later passes make constant or that fold by type, and still re-infer when they do. The lowered tree and wireup are byte-identical to main's apart from branchfold.go itself. pkg/compiler is untouched; *ir-compile* and gogen_ir both run optimize-fn and so both get the earlier fold. A new test fails if a constant test is left for branchfold-1 again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8733bba to
9b2455c
Compare
The
Expensive lowering e2estep has a 600 s timeout, and since #1087 it runs close to that on the slower runners:TestLoweringDeterminismwent from about 400 s to about 480 s there, and the step from about 490 s to 550–580 s. #1089 timed out in it twice. Most of the added time is branch folding. Profiled per pass, one lowering of core spent 16.6 s inbranchfold-1/-2:known-truthranarg-derived?on every branch-if test before reading the test's type, and that trace walks block params back through their predecessors and resolves callee vars. Its answer only matters when the type would fold, and in core no test folds by type.(if :else x nil)a macro can leave, which need no types, but each waited for the first epoch and then paid for a second one from scratch.This change:
known-truthand runsarg-derived?only when the type would fold, which gives the same answers;branchfold-constants, run asbranchfold-0before the first typeinfra epoch. It folds constant tests only, so the first epoch infers the folded CFG once.branchfold-1andbranchfold-2are unchanged: they still fold tests that later passes make constant or that fold by type, and re-infer when they do.In core this leaves no re-inference, and branch folding costs 1.9 s instead of 16.6 s.
TestOptimizeFnFoldsConstantBranchesBeforeTypesfails if a constant test is left forbranchfold-1again.A further step would be for typeinfer to treat an edge whose test is known as dead, SCCP-style. Type-based folds would then need no re-epoch either, and
branchfold-0could go. That changes typeinfer itself, so I left it out here; none of core's folds are type-based today.Compile paths:
pkg/compileris untouched (#1087 gave it its own literal-test skip).*ir-compile*and-tags gogen_irboth runoptimize-fn, so both get the earlier fold.uniform-pass-listkeepsbranchfold-0too; it has no epochs, so this changes nothing there beyond folding sooner.Verification
lgbgen --target=goare byte-identical tomain's, apart frombranchfold.goitself and the tree checksum.lgbgen --target=goon core, interleaved withmain, Apple M2: rounds 2 and 3 took 103.1 / 103.3 s onmainand 89.3 / 87.1 s here (round 1, 138.0 / 94.0 s, was cold).BenchmarkIRCompile, 8 interleaved rounds: allocs/op −2.9% (bytecode) and −2.4% (gogen_ir), B/op −3.3% / −2.9%, p=0.000. sec/op is within noise; its corpus has few constant tests.make bench-ratchethas no timing baseline for this machine, so it gated allocs and bytes only: OK.make parity-full: all three suites parity-identical.make test,make check-generated,go vet ./pkg/....branchfold-0removed from the list (1 fold left forbranchfold-1) and passes with it, with and without-tags gogen_ir.Expensive lowering e2estep on this PR: 311 s, withTestLoweringDeterminismat 245.6 s. That is one run, most likely on a fast runner (those took 220–233 s for the test before feat(ir): fold branches with a known test; infer self-recursive returns #1087); since feat(ir): fold branches with a known test; infer self-recursive returns #1087 the step has taken 550–580 s on slow runners.GOOS=js GOARCH=wasm go vet ./pkg/...fails onmaintoo (pkg/rt/net_test.go:unboxNetConnundefined); this change doesn't touch it.🤖 Generated with Claude Code