From c4e729ab4ef65d29a055ac03bc6f71a722d71e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20G=C3=B6rgens?= Date: Wed, 11 Dec 2024 06:17:01 +0800 Subject: [PATCH 1/3] Remove redundant format Make targets (#724) Just use `cargo fmt` or `cargo fmt --all` for the same effect. Especially the `fmt` target was obsolote: why would you want to format specifically only `ceno_zkvm`? --- .github/workflows/lints.yml | 2 +- Makefile.toml | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/.github/workflows/lints.yml b/.github/workflows/lints.yml index f87d32886..1014ba9a1 100644 --- a/.github/workflows/lints.yml +++ b/.github/workflows/lints.yml @@ -63,7 +63,7 @@ jobs: run: | cargo make --version || cargo install cargo-make - name: Check code format - run: cargo make fmt-all-check + run: cargo fmt --all --check - name: Run clippy env: diff --git a/Makefile.toml b/Makefile.toml index b9f231db4..8d6ab5edf 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -33,26 +33,11 @@ args = [ command = "cargo" workspace = false -[tasks.fmt-all-check] -args = ["fmt", "--all", "--", "--check"] -command = "cargo" -workspace = false - -[tasks.fmt-all] -args = ["fmt", "--all"] -command = "cargo" -workspace = false - [tasks.clippy-all] args = ["clippy", "--all-features", "--all-targets", "--", "-D", "warnings"] command = "cargo" workspace = false -[tasks.fmt] -args = ["fmt", "-p", "ceno_zkvm", "--", "--check"] -command = "cargo" -workspace = false - [tasks.riscv_stats] args = ["run", "--bin", "riscv_stats"] command = "cargo" From a879b846a48acbb6142f88dd0eba768059e5b439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20G=C3=B6rgens?= Date: Wed, 11 Dec 2024 10:25:57 +0800 Subject: [PATCH 2/3] Rename `mod divu` to `mod div` (#733) To make the diff for https://github.com/scroll-tech/ceno/pull/596 easier to read. --- ceno_zkvm/src/instructions/riscv.rs | 2 +- ceno_zkvm/src/instructions/riscv/{divu.rs => div.rs} | 2 +- ceno_zkvm/src/instructions/riscv/rv32im.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename ceno_zkvm/src/instructions/riscv/{divu.rs => div.rs} (99%) diff --git a/ceno_zkvm/src/instructions/riscv.rs b/ceno_zkvm/src/instructions/riscv.rs index 0c8272846..de0dfc771 100644 --- a/ceno_zkvm/src/instructions/riscv.rs +++ b/ceno_zkvm/src/instructions/riscv.rs @@ -11,7 +11,7 @@ pub mod arith_imm; pub mod branch; pub mod config; pub mod constants; -pub mod divu; +pub mod div; pub mod dummy; pub mod ecall; pub mod jump; diff --git a/ceno_zkvm/src/instructions/riscv/divu.rs b/ceno_zkvm/src/instructions/riscv/div.rs similarity index 99% rename from ceno_zkvm/src/instructions/riscv/divu.rs rename to ceno_zkvm/src/instructions/riscv/div.rs index 8d25f583f..e9cb2e4ca 100644 --- a/ceno_zkvm/src/instructions/riscv/divu.rs +++ b/ceno_zkvm/src/instructions/riscv/div.rs @@ -181,7 +181,7 @@ mod test { circuit_builder::{CircuitBuilder, ConstraintSystem}, instructions::{ Instruction, - riscv::{constants::UInt, divu::DivUInstruction}, + riscv::{constants::UInt, div::DivUInstruction}, }, scheme::mock_prover::{MOCK_PC_START, MockProver}, }; diff --git a/ceno_zkvm/src/instructions/riscv/rv32im.rs b/ceno_zkvm/src/instructions/riscv/rv32im.rs index b11cbfee5..d404cd073 100644 --- a/ceno_zkvm/src/instructions/riscv/rv32im.rs +++ b/ceno_zkvm/src/instructions/riscv/rv32im.rs @@ -7,7 +7,7 @@ use crate::{ branch::{ BeqInstruction, BgeInstruction, BgeuInstruction, BltInstruction, BneInstruction, }, - divu::DivUInstruction, + div::DivUInstruction, logic::{AndInstruction, OrInstruction, XorInstruction}, logic_imm::{AndiInstruction, OriInstruction, XoriInstruction}, mul::MulhuInstruction, @@ -27,7 +27,7 @@ use ceno_emul::{ InsnKind::{self, *}, Platform, StepRecord, }; -use divu::{DivDummy, RemDummy, RemuDummy}; +use div::{DivDummy, RemDummy, RemuDummy}; use ecall::EcallDummy; use ff_ext::ExtensionField; use itertools::Itertools; From 2cd6a6d7b713ca7ff77bcebc3bd78dd3049f69d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20G=C3=B6rgens?= Date: Thu, 12 Dec 2024 10:04:43 +0800 Subject: [PATCH 3/3] Introduce `Value::as_i32` (#732) To help make https://github.com/scroll-tech/ceno/pull/596 easier to read and reason about. Also introduce a few more conversion helpers. --- ceno_zkvm/src/instructions/riscv/mul.rs | 4 +-- ceno_zkvm/src/uint.rs | 34 ++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/ceno_zkvm/src/instructions/riscv/mul.rs b/ceno_zkvm/src/instructions/riscv/mul.rs index 58b410960..16a08fe41 100644 --- a/ceno_zkvm/src/instructions/riscv/mul.rs +++ b/ceno_zkvm/src/instructions/riscv/mul.rs @@ -432,9 +432,7 @@ impl Signed { lkm, *val.as_u16_limbs().last().unwrap() as u64, )?; - let signed_val = val.as_u32() as i32; - - Ok(signed_val) + Ok(i32::from(val)) } pub fn expr(&self) -> Expression { diff --git a/ceno_zkvm/src/uint.rs b/ceno_zkvm/src/uint.rs index 193d34f13..5a639a055 100644 --- a/ceno_zkvm/src/uint.rs +++ b/ceno_zkvm/src/uint.rs @@ -606,6 +606,30 @@ pub struct Value<'a, T: Into + From + Copy + Default> { pub limbs: Cow<'a, [u16]>, } +impl<'a, T: Into + From + Copy + Default> From<&'a Value<'a, T>> for &'a [u16] { + fn from(v: &'a Value<'a, T>) -> Self { + v.as_u16_limbs() + } +} + +impl<'a, T: Into + From + Copy + Default> From<&Value<'a, T>> for u64 { + fn from(v: &Value<'a, T>) -> Self { + v.as_u64() + } +} + +impl<'a, T: Into + From + Copy + Default> From<&Value<'a, T>> for u32 { + fn from(v: &Value<'a, T>) -> Self { + v.as_u32() + } +} + +impl<'a, T: Into + From + Copy + Default> From<&Value<'a, T>> for i32 { + fn from(v: &Value<'a, T>) -> Self { + v.as_i32() + } +} + // TODO generalize to support non 16 bit limbs // TODO optimize api with fixed size array impl<'a, T: Into + From + Copy + Default> Value<'a, T> { @@ -616,10 +640,7 @@ impl<'a, T: Into + From + Copy + Default> Value<'a, T> { const LIMBS: usize = (Self::M + 15) / 16; pub fn new(val: T, lkm: &mut LkMultiplicity) -> Self { - let uint = Value:: { - val, - limbs: Cow::Owned(Self::split_to_u16(val)), - }; + let uint = Self::new_unchecked(val); Self::assert_u16(&uint.limbs, lkm); uint } @@ -684,6 +705,11 @@ impl<'a, T: Into + From + Copy + Default> Value<'a, T> { self.as_u64() as u32 } + /// Convert the limbs to an i32 value + pub fn as_i32(&self) -> i32 { + self.as_u32() as i32 + } + pub fn u16_fields(&self) -> Vec { self.limbs.iter().map(|v| F::from(*v as u64)).collect_vec() }