-
Notifications
You must be signed in to change notification settings - Fork 38
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
feat: Add INCR
privileged instructions
#734
base: develop
Are you sure you want to change the base?
Changes from 11 commits
82c5982
cca978f
1270181
c69d9e9
b99ed9c
89caabf
a848075
b45751c
f444249
58cf528
7c765fd
b786905
38fbe0e
b7105bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some constraints seem to be missing, and some seem to be unneeded. The current set of constraints work as intended for I think the clean way to do it is to filter all of the current constraints with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This would be catching INCR3 too, which wouldn't work. If I use both bits I'll get a degree 4, but I can introduce helper columns. I'll look at it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
use plonky2::field::extension::Extendable; | ||
use plonky2::field::packed::PackedField; | ||
use plonky2::field::types::Field; | ||
use plonky2::hash::hash_types::RichField; | ||
use plonky2::iop::ext_target::ExtensionTarget; | ||
use plonky2::plonk::circuit_builder::CircuitBuilder; | ||
use starky::constraint_consumer::{ConstraintConsumer, RecursiveConstraintConsumer}; | ||
|
||
use super::dup_swap::{constrain_channel_ext_circuit, constrain_channel_packed}; | ||
use crate::cpu::columns::CpuColumnsView; | ||
|
||
/// Evaluates the constraints for the DUP and SWAP opcodes. | ||
Nashtare marked this conversation as resolved.
Show resolved
Hide resolved
Nashtare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pub(crate) fn eval_packed<P: PackedField>( | ||
lv: &CpuColumnsView<P>, | ||
nv: &CpuColumnsView<P>, | ||
yield_constr: &mut ConstraintConsumer<P>, | ||
) { | ||
let filter = lv.op.incr; | ||
|
||
let n = lv.opcode_bits[0] | ||
+ lv.opcode_bits[1] * P::Scalar::from_canonical_u64(2) | ||
+ lv.opcode_bits[2] * P::Scalar::from_canonical_u64(4); | ||
Nashtare marked this conversation as resolved.
Show resolved
Hide resolved
Nashtare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Disable the partial channel. | ||
yield_constr.constraint(lv.op.incr * lv.partial_channel.used); | ||
|
||
// Constrain the input channel's address, `is_read` and `used` fields. | ||
let read_channel = &lv.mem_channels[1]; | ||
constrain_channel_packed(true, filter, n, read_channel, lv, yield_constr); | ||
|
||
// Constrain the output channel's address, `is_read` and `used` fields. | ||
let write_channel = &lv.mem_channels[2]; | ||
constrain_channel_packed(false, filter, n, write_channel, lv, yield_constr); | ||
|
||
// Constrain the unchanged stack len. | ||
yield_constr.constraint_transition(filter * (nv.stack_len - lv.stack_len)); | ||
} | ||
|
||
/// Circuit version of `eval_packed`. | ||
/// Evaluates the constraints for the DUP and SWAP opcodes. | ||
pub(crate) fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>( | ||
builder: &mut CircuitBuilder<F, D>, | ||
lv: &CpuColumnsView<ExtensionTarget<D>>, | ||
nv: &CpuColumnsView<ExtensionTarget<D>>, | ||
yield_constr: &mut RecursiveConstraintConsumer<F, D>, | ||
) { | ||
let filter = lv.op.incr; | ||
|
||
let n = lv.opcode_bits[..3].iter().enumerate().fold( | ||
builder.zero_extension(), | ||
|cumul, (i, &bit)| { | ||
builder.mul_const_add_extension(F::from_canonical_u64(1 << i), bit, cumul) | ||
}, | ||
); | ||
|
||
// Disable the partial channel. | ||
{ | ||
let constr = builder.mul_extension(lv.op.incr, lv.partial_channel.used); | ||
yield_constr.constraint(builder, constr); | ||
} | ||
|
||
// Constrain the input channel's address, `is_read` and `used` fields. | ||
let read_channel = &lv.mem_channels[1]; | ||
constrain_channel_ext_circuit(builder, true, filter, n, read_channel, lv, yield_constr); | ||
|
||
// Constrain the output channel's address, `is_read` and `used` fields. | ||
let write_channel = &lv.mem_channels[2]; | ||
constrain_channel_ext_circuit(builder, false, filter, n, write_channel, lv, yield_constr); | ||
|
||
// Constrain the unchanged stack len. | ||
{ | ||
let diff = builder.sub_extension(nv.stack_len, lv.stack_len); | ||
let constr = builder.mul_extension(filter, diff); | ||
yield_constr.constraint_transition(builder, constr); | ||
} | ||
} |
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.
A possible optimization to avoid introducing an extra CTL just for
INCR1
is to keep the same arithmetic CTL for all cases, with memory channels 1 and 2. Then you add INCR1-specific constraints to check thatlv.stack_top == mem_channel[1]
andnv.stack_top == mem_channel[2]
.It requires using the value columns of some disable channels, but I couldn't find anywhere in the code that it wasn't possible.