Skip to content
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

WIP: feat: support uniarg to reduce instruction number #299

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

junyu0312
Copy link
Collaborator

No description provided.

@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch 2 times, most recently from d8a1254 to 7300281 Compare November 4, 2024 07:51
@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch 4 times, most recently from d913162 to 34043f2 Compare November 12, 2024 08:56
let d = allocator.alloc_u64_cell();
let d_flag_helper_diff = allocator.alloc_common_range_cell();
let d_flag_helper_diff = allocator.alloc_common_range_cell(); // TODO: u16??
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

todo

@junyu0312
Copy link
Collaborator Author

junyu0312 commented Nov 14, 2024

  • order of uniargs(uniarg_configs[0] is rhs, uniarg_configs[1] is lhs)
  • common range should be replaced with u16 as possible
  • Is replacing alloc_memory_table_lookup_read_cell with CommonConfig reserves all constraints?(eid in [start, end), is_i32, and?)
  • Is is_i32_cell constraint necessary?(for example, select: uniarg)
  • constants in image table(permutation, lookup...)
    • a new fixed col opcode_prefix in image table, 1 indicates constant, 2 indicates instruction
    • etable lookups instruction if (row % 4 == 0), otherwise lookups constant

@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch 2 times, most recently from 347d0a9 to 551b6fa Compare November 14, 2024 16:23
@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch 2 times, most recently from b417ded to 4005c32 Compare November 15, 2024 07:32
@junyu0312
Copy link
Collaborator Author

Add a u16 column to replace common range with u16 as possible, thus I should remove a common range col

@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch 8 times, most recently from 0f53a8c to d2f9cf9 Compare November 19, 2024 06:56
@junyu0312 junyu0312 force-pushed the zhangjunyu/rebase_main_uniarg branch from d2f9cf9 to 4a46c6d Compare November 19, 2024 09:09
const INDEX_SHIFT: u32 = DROP_SHIFT + COMMON_RANGE_OFFSET;
const DROP_SHIFT: u32 = KEEP_SHIFT + COMMON_RANGE_OFFSET;
const KEEP_SHIFT: u32 = DST_PC_SHIFT + COMMON_RANGE_OFFSET;
const FID_SHIFT: u32 = IID_SHIFT + IID_BITS;

Choose a reason for hiding this comment

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

Add assertion that IID_BITS, FID_BITS are one of (u8, u16, ucommon), so we know they are limited by range check in circuit.

impl<const N: usize> From<&[UniArg; N]> for UniArgEncode<N, BigUint> {
fn from(uniargs: &[UniArg; N]) -> Self {
UniArgEncode::Value(
uniargs

Choose a reason for hiding this comment

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

directly map on array

}
}

pub fn try_decease_stack_depth(&mut self, diff: usize) {

Choose a reason for hiding this comment

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

typo

BigUint::from(3u64) << 64
}

pub fn i64_i32_const_tag() -> BigUint {

Choose a reason for hiding this comment

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

maybe call it tag_diff

matches!(self, UniArg::Pop)
}

pub fn get_const_value(&self) -> u64 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return option

@@ -17,10 +17,11 @@ pub struct InitializationState<T> {
pub maximal_memory_pages: T,
}

pub const INITIALIZATION_STATE_FIELD_COUNT: usize = 10;
impl<T> InitializationState<T> {
// TODO: try to remove the magic number
Copy link
Collaborator Author

@junyu0312 junyu0312 Nov 21, 2024

Choose a reason for hiding this comment

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

comment, or sizeof

@junyu0312
Copy link
Collaborator Author

rename COMMON_RANGE_BITS to U32_BITs

* remove TODO.md
* fix UniargEncode From trait
* fix typo
* rename i64_i32_const_tag, COMMON_RANGE_BITS
* refactor get_const_value
* comment INITIALIZATION_STATE_FIELD_COUNT by code
let memory_table_lookup_stack_read_return_value = allocator
.alloc_memory_table_lookup_read_cell_with_value(
"op_br_if_eqz stack read return value",
constraint_builder,
eid,
move |____| constant_from!(LocationType::Stack as u64),
move |meta| sp.expr(meta) + constant_from!(2),
move |meta| Self::sp_after_uniarg(sp, &uniarg_configs, meta) + constant_from!(1),

Choose a reason for hiding this comment

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

sp constrained by unused uniarg_configs

Choose a reason for hiding this comment

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

not bug because uniarg_configs' size is limited.

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.

3 participants