ssa: use defer selectors on all targets (superseded by #2208) - #2270
ssa: use defer selectors on all targets (superseded by #2208)#2270cpunion wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Review: generic static defer dispatch
Clean, well-motivated refactor. Replacing blockaddress/indirectbr defer dispatch with dense integer continuation selectors dispatched via switch is a net codegen improvement (enables jump-table lowering and unblocks CFG passes), the reserved-selector scheme is documented at both the definition and consumption sites, and the unreachable default is the correct pattern. I traced the rethChain/rethTargets/rundTargets index math and the selectors stay dense and consistent with the emitted switch cases — no off-by-one in the selector math. Good coverage of the tricky RunDefers-before-Defer and i32/i64 switch-width cases in the new test.
A few findings below (2 inline).
Dead code no longer reachable but left in tree
ssa/stmt_builder.go:212—Builder.IndirectJump(emitsCreateIndirectBr) has no remaining callers after this PR removes its only uses fromssa/eh.go. Its companionaBasicBlock.Addr()(ssa/stmt_builder.go:52, emitsblockaddress) also appears to have no live callers now (all other.Addr()hits resolve tonet/reflect/netip). Since the PR's stated goal is to eliminate defer-generatedblockaddress/indirectbr, leaving the helpers that emit them invites accidental reuse — consider removing them here (after confirming no external/generated caller depends onAddr()).
Loop-defer drainer not converted to switch dispatch (optional)
ssa/eh.go:452-473— the reth/rund continuations now useswitch, butloopDeferDrainerstill dispatches onnodeIDwith a hand-rolled linear chain of equality tests, emitting 2 extra basic blocks per case and an O(k) compare chain per drained node in the drain loop. Thec.idvalues are dense integer constants, so this is exactly the shapeCreateSwitch/jumpDeferTargethandles. Not a regression (this path is unchanged by the PR), but aligning it with the new dispatch strategy would be more consistent and lower IR size. Optional follow-up.
_Reviewed ssa/eh.go, runtime/internal/runtime/z_rt.go, ssa/eh_defer_test.go, and the cl/testgo CHECK updates.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Follow-up on the two optional/general review notes:
|
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared only with the latest matching platform in the main series. |
a830839 to
2353dec
Compare
|
Superseded by #2208, which now contains the tested Wasm-only Rund/Reth selector design while preserving Native defer dispatch. |
Superseded by #2208.
This PR explored replacing defer continuation block addresses with selectors on every target. After reviewing Native lowering and performance risk, the final design keeps Native
blockaddress/indirectbrcode generation unchanged and applies selector dispatch only to Wasm.The complete Rund and Reth implementation, Native/Wasm lit coverage, runtime execution coverage, and Wasm build validation have been moved to #2208. Closing this PR keeps review and CI focused on the target-specific design.