-
Notifications
You must be signed in to change notification settings - Fork 597
feat(avm)!: WIP remove is_infinite flags from ECADD opcode (outside AVM only)
#23031
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
Changes from all commits
d929900
ecde0c5
492aa7f
faf94ad
fe3d77e
5ab28e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1280,32 +1280,28 @@ fn handle_black_box_function( | |
| BlackBoxOp::EmbeddedCurveAdd { | ||
| input1_x: p1_x_offset, | ||
| input1_y: p1_y_offset, | ||
| input1_infinite: p1_infinite_offset, | ||
| input1_infinite: _, | ||
|
Contributor
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. is this change waiting on upstream fixes in noir/brillig?
Contributor
Author
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. Pretty much - if I remove it there are cascading changes required in ACIR (IIRC), so I left it unused for now |
||
| input2_x: p2_x_offset, | ||
| input2_y: p2_y_offset, | ||
| input2_infinite: p2_infinite_offset, | ||
| input2_infinite: _, | ||
| result, | ||
| } => avm_instrs.push(AvmInstruction { | ||
| opcode: AvmOpcode::ECADD, | ||
| // The result (SIXTH operand) is indirect (addressing mode). | ||
| // The result (FOURTH operand) is indirect (addressing mode). | ||
| addressing_mode: Some( | ||
| AddressingModeBuilder::default() | ||
| .direct_operand(p1_x_offset) | ||
| .direct_operand(p1_y_offset) | ||
| .direct_operand(p1_infinite_offset) | ||
| .direct_operand(p2_x_offset) | ||
| .direct_operand(p2_y_offset) | ||
| .direct_operand(p2_infinite_offset) | ||
| .indirect_operand(&result.pointer) | ||
| .build(), | ||
| ), | ||
| operands: vec![ | ||
| AvmOperand::U16 { value: p1_x_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: p1_y_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: p1_infinite_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: p2_x_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: p2_y_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: p2_infinite_offset.to_u32() as u16 }, | ||
| AvmOperand::U16 { value: result.pointer.to_u32() as u16 }, | ||
| ], | ||
| ..Default::default() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -451,7 +451,7 @@ export const AVM_DEBUGLOG_BASE_L2_GAS = 9; | |
| export const AVM_POSEIDON2_BASE_L2_GAS = 360; | ||
| export const AVM_SHA256COMPRESSION_BASE_L2_GAS = 12288; | ||
| export const AVM_KECCAKF1600_BASE_L2_GAS = 58176; | ||
| export const AVM_ECADD_BASE_L2_GAS = 270; | ||
| export const AVM_ECADD_BASE_L2_GAS = 180; | ||
| export const AVM_TORADIXBE_BASE_L2_GAS = 24; | ||
| export const AVM_CALLDATACOPY_DYN_L2_GAS = 3; | ||
| export const AVM_RETURNDATACOPY_DYN_L2_GAS = 3; | ||
|
|
@@ -497,6 +497,7 @@ export const GRUMPKIN_ONE_Y = 17631683881184975370165255887551781615748388533673 | |
| export const DEFAULT_MAX_DEBUG_LOG_MEMORY_READS = 125000; | ||
| export enum DomainSeparator { | ||
| NOTE_HASH = 116501019, | ||
| PARTIAL_NOTE_COMMITMENT = 568912195, | ||
|
Contributor
Author
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. I think this is just leftover from some previous changes not updating constants - unrelated to this work! |
||
| SILOED_NOTE_HASH = 3361878420, | ||
| UNIQUE_NOTE_HASH = 226850429, | ||
| NOTE_HASH_NONCE = 1721808740, | ||
|
|
||
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.
Note: to avoid a large diff/confusion, rather than shift all the addresses when removing inf I just left gaps (e.g.
d24). Happy to remove these gaps though!