-
Notifications
You must be signed in to change notification settings - Fork 3
Implement std::hint::black_box intrinsic support in KMIR #659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This commit introduces a new test file `intrinsic-blackbox.rs` that demonstrates the use of `std::hint::black_box` to prevent compiler optimizations on the `add_one` function. The test asserts that the function correctly returns the expected value when wrapped in `black_box`, ensuring its execution is preserved during optimization.
This commit introduces a new Makefile target `generate-tests-smir` for generating SMIR JSON files from Rust source files located in the `tests/rust/` directory. It also adds a `clean-tests-smir` target to remove generated files and a `regenerate-tests-smir` target to clean and regenerate all SMIR JSON files. Additionally, new utility functions in `utils.py` facilitate the retrieval of test files and expected output paths for testing. A new test file `test_smir.py` is added to validate the `function_symbols_reverse` functionality using the generated SMIR JSON data. The README is updated to include instructions for adding new tests and using the Makefile targets.
This commit refactors the test utilities in `utils.py` by adding a new function `filter_test_files` to filter test files based on regex patterns. It also updates the `get_test_files` function for improved readability. In `test_smir.py`, the test structure is modified to utilize the new filtering function, allowing for more flexible test parameterization. The `_test_smir_property` function is introduced to streamline the testing of SMIR properties, enhancing code maintainability and clarity.
This commit refactors the `_make_function_map` method in the `KMIR` class to return a dictionary of function types and their corresponding bodies, improving clarity and usability. The method is renamed to `functions` to better reflect its purpose. Additionally, a new private method `_make_function_map` is introduced to facilitate the mapping of types to their bodies using the refactored `functions` method. In `SMIRInfo`, the `function_symbols_reverse` method is updated to ensure it correctly handles symbols with multiple types, enhancing robustness. New integration and unit tests are added to validate the functionality of the refactored methods using actual SMIR JSON data, ensuring comprehensive coverage and correctness.
- Add IntrinsicFunction(MIRString) variant to MonoItemKind syntax - This enables representation of compiler intrinsic functions in K semantics - Required for implementing std::hint::black_box and other intrinsics 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Enhance _make_function_map to handle IntrinsicSym entries - Automatically detect and map intrinsic functions from SMIR data - Create IntrinsicFunction entries for functions with IntrinsicSym - Enables Python layer to properly recognize compiler intrinsics 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add #getBlocksAux rule for IntrinsicFunction pattern matching - Implement #setUpCalleeData rule for intrinsic function calls - Add #execIntrinsic syntax and infrastructure for intrinsic execution - Implement std::hint::black_box as identity function - Add comprehensive documentation section for intrinsic functions - Support bypassing normal function call setup for intrinsics 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add all tests from /tests directory to EXEC_DATA - Include blackbox.rs intrinsic test in integration testing - Use TEST_FILES from utils for comprehensive test coverage - Enable automated testing of std::hint::black_box implementation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add comprehensive execution state for black_box test case - Shows successful intrinsic execution with value 11 passed through - Validates black_box identity function semantics - Ensures test regression detection for intrinsic functionality 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add CLAUDE.md with essential project setup instructions - Include documentation about reading docs/ before actions - Specify file update requirements for docs/ - Add git commit guidelines for state preservation - Establish coding standards and best practices 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix flake8 B907: Use !r conversion flag for string formatting - Apply black formatting to Python files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Create docs/dev/ directory for developer guides - Add comprehensive guide for adding new intrinsics with correct workflow - Add testing guide with common commands and patterns - Document the iterative development process for intrinsic implementation - Include examples and expected behavior patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Introduce a section for pre-task preparation, emphasizing the importance of reading documentation and understanding the codebase structure. - Update the Python code modification instructions to include formatting checks before verifying code quality.
- Change the representation of intrinsic functions from MIRString to Symbol in the KMIR codebase. - Modify related documentation to reflect the new syntax for intrinsic function execution and definitions. - Ensure consistency across the semantic definitions for intrinsic functions in both mono and kdist contexts.
- Update test state files to reflect the new representation of intrinsic functions as Symbols instead of MIRStrings. - Ensure consistency across various test cases, including arithmetic, arrays, and references, by adding the black_box intrinsic function. - Modify expected outputs in tests to align with the recent changes in intrinsic function handling. This change enhances the accuracy of intrinsic function testing and maintains alignment with the latest updates in the KMIR codebase.
- Refactor the functions method to include parsing of intrinsic functions directly within the KMIR class. - Update the expected output for intrinsic functions in the integration tests to reflect the new parsing logic. - Ensure that intrinsic functions are correctly represented in the test cases, maintaining consistency with recent changes in the KMIR codebase. This update improves the handling and testing of intrinsic functions, aligning with the latest developments in the project.
- Clean up unnecessary whitespace in the functions method of the KMIR class. - Ensure consistent formatting for improved readability and maintainability of the code. This minor update enhances the overall code quality without altering functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please separate moving the test files to the new tests/
directory from the cahnge to implement the black_box
intrinsic and intrinsic support?
CLAUDE.md
Outdated
- `kdist/mir-semantics/` - K semantics definitions | ||
- `src/tests/` - Test suites | ||
- `integration/data/prove-rs/` - Rust test programs for prove-rs | ||
- `integration/data/run-rs/` - Rust programs for execution tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😏 Actually not true... run-rs
is only used for parse tests. exec-smir
is the one that has the execution tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Successfully deceived Claude 😏
# CLAUDE.md | ||
|
||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, just today I signed up for Claude Pro. If you have time we can maybe chat about how you use it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe tomorrow after sync up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Pro for one day and updated to max...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much more better than Cursor.
tests/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you would like to move all exec-smir
tests here?
We should not have tests in two places, though, and the current setup is to have the tests in kmir/src/tests/integration/data
for easy access from python.
Maybe we leave this change for a separate PR if we want to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can move all the exec-smir files over there, if you don't mind me making these changes in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like that move to be a separate PR. It is refactoring and should not be mixed up with new feature work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let me change my testing.
…ction (#657) - Updated documentation to explain how to directly access elements of an Aggregate when #traverseProjection becomes stuck. - Introduced a new rule for handling single-element aggregates in the semantics. - Added a new test case for closure access to a struct, ensuring correct behavior. - Updated integration tests to include the new failure case for closure access to structs.
8298d4b
to
26303ce
Compare
- Update CLAUDE.md to reflect changes in test directories. - Remove the utils.py file as it contained unused functions for test file management. - Modify integration and unit tests to directly reference test data without relying on the removed utility functions. - Adjust expected output paths in tests to align with the new structure. This refactor streamlines the test organization and improves maintainability by eliminating unnecessary code.
…seProjection - Removed outdated explanations regarding direct access to single-element aggregates. - Clarified the importance of the new rule for unwrapping single-element aggregates in the context of closures accessing struct fields. - Enhanced the overall clarity of the documentation to better reflect the current behavior and requirements. This update ensures that the documentation accurately represents the functionality and rationale behind the changes made in previous commits.
- Deleted the testing.md file, which contained instructions and structure for running unit and integration tests. - This removal reflects the current state of the project and streamlines the documentation by eliminating redundant information. This change helps maintain clarity and relevance in the project's documentation.
- Changed the depth parameter for the 'intrinsic-blackbox' entry in EXEC_DATA from None to 1000. - Adjusted the test parameterization in test_prove_termination to exclude the 'intrinsic-blackbox' name, streamlining the test cases. These updates enhance the clarity and focus of the integration tests, ensuring they accurately reflect the intended test scenarios.
This PR implements support for Rust intrinsic functions in the MIR semantics, starting with the black_box intrinsic. Intrinsic functions are compiler built-ins that don't have regular MIR bodies and require special handling in the formal semantics.
Key Changes
Core Implementation
K Semantics
Testing & Documentation
Implementation Details
The intrinsic function support follows this flow:
Future Work
This PR establishes the foundation for supporting additional Rust intrinsics. The modular design makes it straightforward to add new intrinsics by:
The test infrastructure refactoring also provides a solid foundation for expanding test coverage across the entire MIR semantics implementation.