feat: add value optimization passes (const_prop, algebraic, copy_prop)#30
Merged
Conversation
Add three value optimization passes to the pre-lowering and post-lowering pipeline: **Pre-lowering passes** (run on SSA IR with phi nodes): - const_prop: Constant folding and propagation. Tracks which VarIds hold known constant values and evaluates operations on constants using Wasm arithmetic semantics (via herkos-runtime functions). - algebraic: Algebraic simplifications. Rewrites BinOp instructions when one operand is a known constant and an identity/annihilator rule applies. (e.g., x * 1 = x, x + 0 = x, x & x = x) - copy_prop (pre-lowering): Backward coalescing and forward substitution. Eliminates intermediate copies before phi lowering. **Post-lowering passes** (run on phi-free IR): - copy_prop (post-lowering): Forward substitution of assignments created by lower_phis, reducing redundant variable copies. ## Value optimizations improve: - Constant folding: Wasm instr with const operands evaluated at compile time - Dead code elimination: Const prop may make more instructions side-effect-free - phi simplification: Pre-lowering const prop folds phi(const, const) - Assignment forwarding: Post-lowering copy_prop forwards lower_phis assignments ## Integration: - Both const_prop and algebraic run in optimize_ir (pre) - copy_prop runs in both optimize_ir (pre) and optimize_lowered_ir (post) - Added PartialEq to IrValue to support test assertions - herkos-core now depends on herkos-runtime for float op evaluation ## Test coverage: All unit tests pass for instruction folding, algebraic rules, copy propagation. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…r-e/value-optimizations
…ue-optimizations
Global copy propagation had a bug where it incorrectly substituted variables that were captured via Assign and then later redefined in loops. For example: v20 = Assign(v10) // capture v10 ... v10 = result // v10 is redefined v11 = v20 // should use OLD v10, not NEW v10 When global copy prop replaced all uses of v20 with v10, the v11 = v20 became v11 = v10, but v10 referred to the NEW value after redefinition, breaking the semantics. This bug manifested as fib_i64 returning 512 instead of 55 for fib(10). The fix disables global copy propagation for multi-block functions (which are typical of loops generated from Wasm). Single-block functions still benefit from the optimization, and the remaining copy propagation passes (backward coalescing and forward substitution) handle most cases. Also marked 4 cross-block copy propagation unit tests as ignored since they test functionality that is now disabled. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Global copy propagation had a subtle bug where it would incorrectly substitute variables in loops when those variables were captured via Assign and then later redefined. This caused the fib_i64 test to return 512 instead of 55. Rather than attempt to fix the complex logic with proper data-flow analysis, we remove global copy propagation entirely. The backward coalescing and forward substitution passes in copy_prop.rs are sufficient for most optimization needs. Marked 10 related unit tests as #[ignore] as they specifically test global copy propagation functionality. Future ticket TODO: Re-introduce global copy propagation with proper safety analysis for multi-block functions (loops), ensuring variables in the dependency chain are not redefined between the Assign and their uses. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The formatter had removed the #[ignore] attribute. Re-add it with a comment explaining that global copy propagation tests are ignored since the pass was removed. See GLOBAL_COPY_PROP_TICKET.md for details on re-introducing it safely. Also created GLOBAL_COPY_PROP_TICKET.md with comprehensive details on how to re-introduce global copy propagation with proper data-flow safety analysis. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds three value-based optimization passes that analyze and simplify IR value operations:
Pre-lowering passes (on SSA IR with phi nodes):
x * 1 → x,x & x → x, etc.Post-lowering passes (on phi-free IR):
lower_phisinserted into predecessor blocks, reducing further redundancy.Key improvements:
phi(5, 5) → const(5)is folded before SSA loweringImplementation notes:
Dependencies:
🤖 Generated with Claude Code