Skip to content

RISC-V no-panic (part 2)#736

Merged
nazar-pc merged 5 commits into
mainfrom
riscv-no-panic-part-2
Jul 21, 2026
Merged

RISC-V no-panic (part 2)#736
nazar-pc merged 5 commits into
mainfrom
riscv-no-panic-part-2

Conversation

@nazar-pc

Copy link
Copy Markdown
Owner

More things were already panic-free, just needed a higher inlining limits. Others required minor changes and clarifications to nudge compiler towards the correct conclusion.

Copilot AI review requested due to automatic review settings July 21, 2026 08:25
@nazar-pc
nazar-pc enabled auto-merge July 21, 2026 08:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the effort to make the RISC-V vector interpreter codebase no-panic compatible by removing panicking paths (e.g., expect) and adding no_panic annotations, plus a CI tweak to allow more inlining for the analysis to succeed.

Changes:

  • Replace some expect(...)-based invariants with unwrap_unchecked() guarded by explicit safety comments in widen/narrow helpers.
  • Add #[cfg_attr(feature = "no-panic", no_panic_const::no_panic)] to a number of vector helper entrypoints that previously had TODO markers.
  • Refactor mul/div helpers to use widening_mul and wrapping_{div,rem}; increase CI inline threshold for the no-panic job.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow/zvexx_widen_narrow_helpers.rs Switch sign-extension helpers to take Vsew; remove expect in favor of unwrap_unchecked; add no_panic attribute to execute helpers.
crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow.rs Update callers to new sign_extend_bits signature (notably XLEN scalar handling).
crates/execution/ab-riscv-interpreter/src/v/zvexx/store/zvexx_store_helpers.rs Add no_panic attribute to store execute helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/reduction/zvexx_reduction_helpers.rs Add no_panic attribute to reduction execute helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/perm/zvexx_perm_helpers.rs Add no_panic attribute to permute/slide/merge/compress helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/muldiv/zvexx_muldiv_helpers.rs Refactor multiply-high/div/rem helpers to reduce panic potential and use widening ops; add no_panic attributes.
crates/execution/ab-riscv-interpreter/src/v/zvexx/mask/zvexx_mask_helpers.rs Add no_panic attribute to mask helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/load/zvexx_load_helpers.rs Add no_panic attribute to load helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/fixed_point/zvexx_fixed_point_helpers.rs Add no_panic attribute to fixed-point helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/config/zvexx_config_helpers.rs Add no_panic attribute to vsetvl/vsetivli config helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/carry/zvexx_carry_helpers.rs Add no_panic attribute to carry/borrow helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/v/zvexx/arith/zvexx_arith_helpers.rs Add no_panic attribute to arithmetic/compare helpers (replacing TODO markers).
crates/execution/ab-riscv-interpreter/src/lib.rs Enable the widening_mul nightly feature gate.
crates/execution/ab-riscv-interpreter/Cargo.toml Update feature-notes comment (removing now-addressed TODO item).
.github/workflows/rust.yml Increase LLVM inline threshold for the no-panic check job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow.rs
Comment thread crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow.rs
Comment thread crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow.rs
Comment thread crates/execution/ab-riscv-interpreter/src/v/zvexx/widen_narrow.rs
Copilot AI review requested due to automatic review settings July 21, 2026 08:59
@nazar-pc
nazar-pc force-pushed the riscv-no-panic-part-2 branch from 22399de to 4482527 Compare July 21, 2026 08:59
@nazar-pc
nazar-pc force-pushed the riscv-no-panic-part-2 branch from 4482527 to 7ca92fc Compare July 21, 2026 09:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Comment thread crates/execution/ab-riscv-primitives/src/instructions/v.rs Outdated
Copilot AI review requested due to automatic review settings July 21, 2026 09:06
@nazar-pc
nazar-pc force-pushed the riscv-no-panic-part-2 branch from 7ca92fc to 2835c50 Compare July 21, 2026 09:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

crates/execution/ab-riscv-primitives/src/instructions/v.rs:515

  • Vsew::from_xlen is annotated with #[cfg_attr(feature = "no-panic", no_panic_const::no_panic)] but still contains a panic!() path. Because Reg::XLEN is not statically proven to be only 32/64 for all Register impls, this will likely fail the no-panic check (and contradicts the goal of the PR). Consider making the “impossible” arm panic-free under feature="no-panic" (while keeping a panic in normal builds).
                64 => Self::E64,
                _ => {
                    // TODO: Should have been `unreachable!()`:
                    //  https://github.com/rust-lang/rust/issues/159645
                    panic!("Invalid register width")

Copilot AI review requested due to automatic review settings July 21, 2026 09:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 21, 2026 09:18
@nazar-pc
nazar-pc force-pushed the riscv-no-panic-part-2 branch from 2835c50 to c3715fb Compare July 21, 2026 09:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

@nazar-pc
nazar-pc merged commit e63bf97 into main Jul 21, 2026
66 checks passed
@nazar-pc
nazar-pc deleted the riscv-no-panic-part-2 branch July 21, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants