Skip to content

upgrade cranelift#6331

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:cranelift
Dec 6, 2025
Merged

upgrade cranelift#6331
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:cranelift

Conversation

@youknowone

@youknowone youknowone commented Dec 6, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

close #6291 #6293

Summary by CodeRabbit

  • Chores

    • Updated Cranelift-related dependencies to version 0.126 for improved compatibility and performance.

  • Refactor

    • Enhanced code maintainability through streamlined value handling and improved type consistency in core compilation routines.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Cranelift dependencies were bumped from version 0.119 to 0.126 in crates/jit/Cargo.toml. The codebase was updated to accommodate API changes: variable allocation now uses builder.declare_var() directly, and control-flow operations (jumps, branches, block parameters) now use .into() conversions for type compatibility with the new Cranelift APIs.

Changes

Cohort / File(s) Summary
Dependency Version Bump
crates/jit/Cargo.toml		 Updated Cranelift-related dependencies (cranelift, cranelift-jit, cranelift-module) from 0.119.x to 0.126.0
API Migration for Cranelift Compatibility
crates/jit/src/instructions.rs		 Replaced manual Variable creation with builder.declare_var() calls; updated IR parameter passing throughout control-flow operations (jumps, branches, block parameters) to use .into() conversions for value type compatibility across exponentiation, arithmetic, and edge-case handling code paths

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Variable declaration refactoring: Verify builder.declare_var() usage is correct throughout store_variable and related allocation paths
  • .into() conversion propagation: Ensure all jump, brif, and block parameter calls properly convert values; multiple affected locations increase cognitive load despite using consistent patterns
  • Edge cases in arithmetic paths: Pay close attention to compile_fpow, ipow, and double-double arithmetic branches where value conversions were applied, as errors here could cause type mismatches or runtime failures

Poem

🐰 Cranelift climbs up, from one-nineteen to one-twenty-six so new,
Variables declared with builders, .into() conversions flow true,
Jumps and branches march in sync, each block parameter now aligned,
Our arithmetic paths sparkle bright, no type mismatch left behind! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'upgrade cranelift' clearly and concisely summarizes the main change of upgrading Cranelift dependencies from 0.119 to 0.126.
Linked Issues check ✅ Passed The PR successfully bumps cranelift-related dependencies (cranelift, cranelift-jit, cranelift-module) from 0.119 to 0.126, meeting the objective in #6291. Code changes in instructions.rs adapt to API changes from the upgraded Cranelift version.
Out of Scope Changes check ✅ Passed All changes are within scope: Cargo.toml updates Cranelift dependencies and instructions.rs adapts code to new Cranelift APIs. Both are necessary parts of the upgrade and directly address the linked issue requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89bcae7 and 0caaede.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • crates/jit/Cargo.toml (1 hunks)
  • crates/jit/src/instructions.rs (17 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/jit/src/instructions.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Check Rust code with clippy
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Run tests under miri
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run rust tests (macos-latest)
🔇 Additional comments (7)
crates/jit/src/instructions.rs (6)

1021-1023: Consistent .into() conversion for jump block parameters.

The .into() conversion is correctly applied to all jump targets passing values to merge_block. This appears to be a type compatibility change required by the Cranelift 0.126 API for block parameters.

1125-1131: Correct brif parameter conversion for parity branching.

The .into() conversions are correctly applied to both branch target parameters, maintaining the logic to pass neg_inf_f to the odd block and inf_f to the even block.

1133-1139: Block parameter handling and jumps are correct.

The pattern of retrieving block parameters and jumping to merge block with .into() conversion is consistent throughout.

1196-1213: Negative base handling with magnitude value propagation is correct.

The brif and subsequent jumps correctly propagate mag_val to both branches, with the odd branch negating the result before merging. The .into() conversions are consistently applied.

1250-1312: Integer power function API migration is correct.

All jump and brif instructions in compile_ipow correctly apply .into() conversions for block parameters. The square-and-multiply algorithm logic is preserved:

  • Initial jump passes exponent and base
  • Negative check branches to handle_negative or loop_block appropriately
  • Loop implements the standard binary exponentiation pattern

113-126: API migration to declare_var returning Variable is correct.

Cranelift 0.126 FunctionBuilder API confirms that declare_var(&mut self, ty: Type) -> Variable returns the Variable directly. The code correctly adapts to this API where builder.declare_var(ty.to_cranelift()) returns the variable, which is then stored in the Local struct. The type validation logic at line 120 is preserved and appropriate.

crates/jit/Cargo.toml (1)

20-22: Version bump looks consistent and correct.

All three Cranelift packages are bumped consistently to version 0.126, which aligns with the PR objective and allows for automatic patch updates (latest available is 0.126.1).

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as ready for review December 6, 2025 00:11
@youknowone youknowone merged commit 56a7fb1 into RustPython:main Dec 6, 2025
13 checks passed
@youknowone youknowone deleted the cranelift branch December 6, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone