cl: deduplicate same-line panic PC anchors - #2360
Conversation
There was a problem hiding this comment.
Review: Deduplicate PC-line metadata sites
This is a clean, well-scoped change. Same-line PC-line anchors within a single SSA basic block are now deduplicated by tracking the last emitted (file, line) on context and resetting it at each compileBlock entry.
Strengths
- Dedup guard in
emitPCLineLabelis O(1) with no new allocations, andpcLineSeqis only incremented after the check passes, so label IDs stay dense and collision-free. - Resetting the anchor at the top of
compileBlockcorrectly prevents dedup from crossing control-flow edges (a jump target must not lose its nearest anchor). This also implicitly resets across functions, so no state leaks. - Dropping the column from the dedup key is correct and intentional: the encoded runtime PC-line record stores only file + line, verified down to
EncodedPCLineRecord. - Test coverage locks both directions: same-line panics in one block collapse to 1 record; the same across separate blocks stay at 2.
- Comments accurately describe the behavior across all changed files.
All four review dimensions (quality, performance, security, documentation) found no blocking issues. One optional maintainability note is inline.
| // last emitted position within one SSA basic block so repeated checks for a | ||
| // single source line can share an anchor. | ||
| lastPCLineFile string | ||
| lastPCLineLine int |
There was a problem hiding this comment.
Optional: the correctness of these dedup fields depends on two invariants — compileBlock resetting them at each block entry, and compilation being single-threaded. Both hold today, but a future refactor that parallelizes compilation would silently corrupt this shared state. Consider a one-line note here recording that dependency so the invariant is visible at the field declaration.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
Summary
Motivation
PR #2293 made implicit panic locations visible to
runtime.Caller, but expressions that lower to several checks on one source line emitted redundant__llgo_pclrecords.In an XGo Darwin A/B build:
__llgo_pcl317,920 bytes__llgo_pcl643,344 bytes__llgo_pcl366,880 bytesThis recovers 705,920 bytes, or 88.9% of the observed binary-size regression, while retaining panic line information.
Testing
go test ./cl -run "TestCompileRuntimeCallerPanicPCLineMetadata|TestRuntimeCallerFuncSetKeepsRecoverObservableCallees|TestRuntimeCallerAnalysisEdgeCases" -count=1go test -vet=off ./test/go -run "TestRuntimeStatementLineInfo|TestRuntimeDeferredPanicLine" -count=1go test -vet=off ./test/go(full package, 244.486s)internal/buildcache tests when the whole run was forced throughLLGO_BUILD_CACHE=off; those four passed after rerunning with the normal cache setting