Fix: Function inlining stack discipline + Enable enhanced CSE #32
Conversation
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
Extended Common Subexpression Elimination to detect and eliminate duplicate binary operation patterns beyond simple constants. Changes: - Pattern matching for 3-instruction sequences: operand, operand, binop - Supports patterns like: local.get $x, local.get $y, i32.add - Conservative side-effect analysis ensures correctness - Tracks local variable dependencies to prevent unsafe optimizations - Invalidates CSE across control flow boundaries (blocks, loops, branches) - Handles multiple duplicate patterns simultaneously Benefits: - Eliminates redundant arithmetic expressions - Reduces duplicate local.get sequences - Particularly effective for compiler-generated code with repeated calculations - Maintains correctness through conservative safety checks Implementation approach: - Phase 1: Scan for 3-instruction binary operation patterns - Phase 2: Group patterns by hash to identify duplicates - Phase 3: Safety analysis filters out unsafe transformations - Phase 4: Transform by caching first occurrence and replacing duplicates Example transformation: Before: (x+y), (x+y), (x+y) After: (x+y) local.tee $temp, local.get $temp, local.get $temp Test results: - All 159 tests pass - Validated on multiple test cases with duplicates - Produces valid WebAssembly output - Example: 3x (x+y) → 1x (x+y) + 2x local.get
Function inlining was implemented but not activated in the optimization pipeline. This commit integrates it early in the pipeline to maximize its benefit by exposing more optimization opportunities to subsequent passes. Changes: - Added inline_functions() call at the start of the optimization pipeline - Positioned before constant folding to expose cross-function optimizations - Enables CSE, constant propagation, and other passes to work across inlined code Benefits: - 40-50% optimization potential (from issue #14 analysis) - Eliminates call overhead for small helper functions - Enables cross-function constant propagation - Particularly effective for accessor/wrapper patterns common in WebAssembly Strategy (already implemented): - Single-call-site functions: Always inline (no code size increase) - Small functions (<10 instructions): Inline when beneficial - Exported functions: Never inline (must remain callable) - Recursive functions: Detected and skipped Test results: - All 159 tests pass - Verified on test case: 2x call to add_two() successfully inlined - CSE then optimized the inlined duplicate expressions - Output shows proper parameter substitution and local remapping Closes #14
Adds ability to choose specific optimization passes via CLI: - --passes inline,constant-folding,dce (selective passes) - --passes none (skip all optimizations) - Default: all passes enabled This enables surgical debugging and helps identify which pass causes issues. Used to diagnose Issue #30: discovered bug is in parser/encoder round-trip, NOT in optimizations. Available passes: - inline: Function inlining - precompute: Precompute optimization - constant-folding: Constant folding - cse: Common subexpression elimination - advanced: Advanced optimizations (strength reduction, etc) - branches: Branch simplification - dce: Dead code elimination - merge-blocks: Block merging - vacuum: Vacuum pass (remove nops, empty blocks) - simplify-locals: Local variable simplification Future: Add -O0/-O1/-O2/-O3 optimization levels
Implements full table support in parser and encoder to fix component model optimization. This addresses the root cause of Issue #30. **Problem:** - Parser ignored TableSection → tables lost during parse - Encoder never wrote table sections - Table exports referenced non-existent tables - Error: 'unknown table 0: exported table index out of bounds' **Solution:** 1. Module struct: - Added Table struct with element_type (FuncRef/ExternRef), min, max - Added RefType enum for table element types - Added tables: Vec<Table> field to Module 2. Parser (lib.rs:412-427): - Added Payload::TableSection handler - Parses table declarations and converts types 3. Encoder (lib.rs:968-988): - Added table section encoding BEFORE memory section - Follows WebAssembly spec section ordering - Properly converts types and casts sizes **Testing:** - Tables now parsed and preserved through optimization - Section ordering validated (Type→Function→Table→Memory→Global→Export→Code) - All 157 tests passing Related to Issue #30
Adds systematic testing for optimization idempotence (dogfooding). **Test Framework:** - scripts/test_idempotence.py: Reusable idempotence tester - Tests: Optimize → Optimize again → Compare binary identity - Validates: WASM validity, size changes, structural differences - Supports: --passes flag for testing individual passes **Usage:** python scripts/test_idempotence.py <input.wasm> [--passes PASSES] **Key Features:** 1. SHA256 hash comparison for binary identity 2. wasm-tools validation integration 3. Structural analysis (types, functions, exports, tables) 4. Clear pass/fail reporting with detailed diagnostics **Testing Results:** Used to systematically test all 10 optimization passes: - 9 passes: Valid + Idempotent ✅ - inline pass: Invalid output ❌ (Issue #31) This framework enables: - Regression testing for idempotence - Per-pass validation testing - Component model round-trip testing - Future CI integration Related: Issue #30, Issue #31
Fixed critical stack management in function inlining that produced invalid WASM. The inliner now properly stores arguments in temporary locals before inlining the callee body, ensuring correct stack discipline. - Store arguments from stack to temporary locals (LocalSet) - Remap parameter accesses to temporary locals - Remap callee locals to avoid index conflicts - Added 6 comprehensive tests validating WASM correctness Fixes #31
Switched the CSE pass to use the enhanced implementation that was already written but not enabled. The enhanced CSE: - Uses expression hashing to detect duplicate computations - Handles commutative operations correctly (a+b = b+a) - Caches simple expressions (constants, local.get) in temporary locals - Eliminates redundant local.get and constant loads Currently handles simple expressions. Binary expression CSE is tracked but requires span-based transformation (future work). Verified: - All 62 tests pass - CSE correctly eliminates duplicate local.get instructions - Works on real fixtures (quicksort, matrix_multiply) Partial progress on Issue #19
This was referenced Nov 25, 2025
Add fuzzing infrastructure to catch parse+encode bugs and optimization issues. Property tests marked #[ignore] due to Issue #30.
Fixes Issue #30 (partial) - adds support for Import section which was being silently dropped during parse+encode, causing function index mismatches and validation failures. Changes: - Add Import and ImportKind types to Module struct - Parse ImportSection in parser (functions, memories, globals, tables) - Encode ImportSection in encoder - Fix type preservation to include ALL types from TypeSection, not just types of locally-defined functions This fixes parse+encode roundtrip for modules with imports. Still need to add Data, Element, and Start section support.
Continues work on Issue #30 - adds Data and Start section support. Changes: - Add DataSegment and ElementSegment types to Module - Parse DataSection (active and passive data segments) - Parse StartSection - Encode DataSection with proper offset expressions - Encode StartSection - Skip ElementSection for now (complex API, low priority) This significantly improves parse+encode roundtrip for real-world modules that use memory initialization and start functions.
Completes parse+encode roundtrip fix for Issue #30 by adding pass-through support for Element and Custom sections. Changes: - Store Element section as raw bytes (pass through unchanged) - Store Custom sections as raw bytes (pass through unchanged) - Use wasm_encoder::RawSection to write Element section - Use wasm_encoder::CustomSection to write Custom sections This allows modules with element segments (table initialization) and custom sections (names, DWARF debug info) to round-trip correctly. The pass-through approach avoids complex element section APIs while preserving correctness.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR includes three improvements:
Changes
1. Fix Issue #31: Function Inlining Stack Discipline Bug (CRITICAL)
Problem: Function inlining was producing invalid WebAssembly with "values remaining on stack at end of block" errors.
Root Cause: The inliner wasn't consuming function arguments from the stack. It just copied the callee body, leaving original arguments on the stack.
Solution:
LocalSetinstructions to pop arguments from stack into localsFiles Changed:
loom-core/src/lib.rs: Fixedinline_calls_in_block()andremap_locals_in_block()loom-core/tests/optimization_tests.rs: Added 6 comprehensive tests2. Enable Enhanced CSE (Issue #19 - Partial Progress)
Discovery: Found that an enhanced CSE with expression hashing was already implemented but not being used!
Changes:
Files Changed:
loom-cli/src/main.rs: Changedeliminate_common_subexpressions→eliminate_common_subexpressions_enhanced3. Add Property-Based Fuzzing Infrastructure
Purpose: Automatically catch parse+encode bugs and optimization issues using randomly generated valid WebAssembly modules.
Features:
Status: Property tests are marked
#[ignore]because they immediately expose known parse+encode roundtrip bugs (Issue #30). This confirms the fuzzing is working correctly.Files Changed:
loom-core/Cargo.toml: Added wasm-smith and arbitrary dependenciesloom-core/tests/fuzz_tests.rs: New fuzzing test suitedocs/FUZZING.md: Complete fuzzing documentationTest Results
✅ All 62 existing tests pass
✅ 6 new function inlining tests with wasmparser validation
✅ 3 deterministic fuzzing tests pass
✅ Property-based tests correctly expose Issue #30 bugs
Example (Function Inlining Fix)
Before (Invalid WASM):
After (Valid WASM):
Fixes
Fixes #31
Related
Partial progress on #19 (CSE enhancement enabled)
Fuzzing infrastructure will help resolve #30