Skip to content
This repository was archived by the owner on Apr 29, 2026. It is now read-only.

⏺ All tests pass. The multi-output specialization is complete on the …#303

Merged
navicore merged 1 commit intomainfrom
multi-output
Jan 25, 2026
Merged

⏺ All tests pass. The multi-output specialization is complete on the …#303
navicore merged 1 commit intomainfrom
multi-output

Conversation

@navicore
Copy link
Copy Markdown
Owner

…multi-output branch. Here's a summary of what was implemented:

Changes in specialization.rs:

  • Removed the single-output restriction - words can now return multiple values
  • Added llvm_return_type() method to SpecSignature that returns { i64, i64 } style struct types for multi-output
  • Updated emit_specialized_return to use LLVM insertvalue instructions to build return structs
  • Updated emit_specialized_recursive_call to handle struct returns
  • Updated emit_specialized_word_dispatch to handle struct returns

Changes in statements.rs:

  • Updated try_specialized_dispatch to use LLVM extractvalue instructions to unpack struct returns and push each value to the virtual stack

Result:
Words like add-and-sub ( Int Int -- Int Int ) now generate efficient register-based LLVM IR:
define { i64, i64 } @seq_add_and_sub_i_i_to_i_i(i64 %arg0, i64 %arg1) {
entry:
%0 = add i64 %arg0, %arg1 %1 = sub i64 %arg0, %arg1 %2 = insertvalue { i64, i64 } undef, i64 %0, 0 %3 = insertvalue { i64, i64 } %2, i64 %1, 1 ret { i64, i64 } %3 }

This completes the register-based specialization feature. The system now handles:

  • Single and multiple inputs
  • Single and multiple outputs (via struct returns)
  • 65 specialized operations
  • Recursive calls
  • Control flow (if/else)

…multi-output branch. Here's a summary of what was implemented:

  Changes in specialization.rs:
  - Removed the single-output restriction - words can now return multiple values
  - Added llvm_return_type() method to SpecSignature that returns { i64, i64 } style struct types for multi-output
  - Updated emit_specialized_return to use LLVM insertvalue instructions to build return structs
  - Updated emit_specialized_recursive_call to handle struct returns
  - Updated emit_specialized_word_dispatch to handle struct returns

  Changes in statements.rs:
  - Updated try_specialized_dispatch to use LLVM extractvalue instructions to unpack struct returns and push each value to the virtual stack

  Result:
  Words like add-and-sub ( Int Int -- Int Int ) now generate efficient register-based LLVM IR:
  define { i64, i64 } @seq_add_and_sub_i_i_to_i_i(i64 %arg0, i64 %arg1) {
  entry:
    %0 = add i64 %arg0, %arg1
    %1 = sub i64 %arg0, %arg1
    %2 = insertvalue { i64, i64 } undef, i64 %0, 0
    %3 = insertvalue { i64, i64 } %2, i64 %1, 1
    ret { i64, i64 } %3
  }

  This completes the register-based specialization feature. The system now handles:
  - Single and multiple inputs
  - Single and multiple outputs (via struct returns)
  - 65 specialized operations
  - Recursive calls
  - Control flow (if/else)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 25, 2026

Code Review: Multi-Output Specialization for Register-Based LLVM IR

Summary

This PR successfully removes the single-output restriction and implements multi-output support for specialized functions using LLVM struct returns. The implementation is clean and follows a consistent pattern across all three key locations (function returns, recursive calls, and word dispatch).


✅ Strengths

  1. Consistent Implementation Pattern: The multi-output handling is uniform across all three code paths:

    • emit_specialized_return (specialization.rs:1410-1461)
    • emit_specialized_recursive_call (specialization.rs:1273-1347)
    • emit_specialized_word_dispatch (specialization.rs:1350-1406)
    • try_specialized_dispatch (statements.rs:103-220)
  2. Proper LLVM IR Generation: The use of insertvalue for building struct returns and extractvalue for unpacking is correct and idiomatic LLVM IR

  3. Good Error Handling: Added validation for context size at return time (specialization.rs:1426-1431)

  4. Backwards Compatibility: Single-output case preserved with no changes, minimizing risk

  5. Type Safety: The llvm_return_type() method (specialization.rs:176-183) properly encapsulates the type string generation


🔍 Areas for Consideration

1. Type Verification Gap (Medium Priority)

In emit_specialized_return (specialization.rs:1434-1436), the code extracts the last N values from context but doesn't verify that their types match sig.outputs. If the context contains values in the wrong order or with mismatched types, the LLVM IR will have a type mismatch.

Recommendation: Add type verification similar to the input validation in try_specialized_dispatch (statements.rs:123-143)

2. Zero-Output Case Handling (Low Priority)

The code at specialization.rs:390-393 rejects zero-output signatures with a comment "zero outputs means side-effect only". However, emit_specialized_return (lines 1415-1416) handles the zero-output case by emitting ret void. Is this intentional or dead code?

3. Code Duplication (Low Priority - Refactoring Opportunity)

The pattern for extracting struct values and pushing to stack/context appears 4 times with slight variations. Consider extracting to a helper method in a future refactoring.

4. Test Coverage (Important)

While the commit message mentions "All tests pass", I don't see test files added/modified in this PR that specifically exercise the multi-output functionality.

Recommendation: Add integration tests with words like add-and-sub ( Int Int -- Int Int ), test edge cases with 3+ outputs, mixed types (i64 and double), and multi-output with recursion.


🔒 Security Considerations

No security concerns identified. The changes don't introduce any unsafe code, maintain type safety through RegisterType, and don't expose any new attack surface.


⚡ Performance Considerations

Excellent work - struct returns are passed via registers on most ABIs (no heap allocation), and the insertvalue/extractvalue pattern is zero-cost in LLVM. Should achieve similar speedups to single-output (8-11x per docs).


📝 Minor Suggestions

  1. Comment Clarity: The comment at specialization.rs:1425 is excellent. Consider adding similar comments to the other extract loops for consistency.

  2. Variable Naming: final_stack_var (statements.rs:183) gets mutated in a loop. Consider current_stack_var for clarity.

  3. Documentation: Consider updating the module-level docs (specialization.rs:1-80) with a multi-output example to match the new capability.


✅ Conclusion

This is solid, production-ready code. The implementation is correct, consistent, and well-structured. The primary recommendation is to add type verification in emit_specialized_return to catch potential bugs earlier and improve test coverage for multi-output scenarios.

Approval Status: ✅ Approved with minor recommendations

@navicore navicore merged commit 9999752 into main Jan 25, 2026
2 checks passed
@navicore navicore deleted the multi-output branch January 25, 2026 18:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant