Skip to content

deduplication for COPY operations #1655

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.

Sign up for GitHub

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

Closed
harsh-ps-2003 wants to merge 9 commits into from
Closed

deduplication for COPY operations #1655

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9d196f3
add copy_cost for *COPY operations
harsh-ps-2003 Jul 24, 2024
6b15eee
seperate common logic of opcodes to a method
harsh-ps-2003 Jul 25, 2024
1b72873
fmt nit
harsh-ps-2003 Jul 25, 2024
0453a50
performance nit
harsh-ps-2003 Jul 26, 2024
9c685ad
fmt nit
harsh-ps-2003 Jul 26, 2024
dbc2d57
test nits
harsh-ps-2003 Jul 26, 2024
1474059
fmt nit
harsh-ps-2003 Jul 26, 2024
a1a488b
refactor
harsh-ps-2003 Aug 6, 2024
f92ff49
Merge branch 'bluealloy:main' into copycode
harsh-ps-2003 Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
performance nit
  • Loading branch information
@harsh-ps-2003
harsh-ps-2003 committed Jul 26, 2024
commit 0453a5015a830202e6611e387499bb405eb19317
22 changes: 12 additions & 10 deletions crates/interpreter/src/instructions/system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ pub fn codecopy<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H)
let len = as_usize_or_fail!(interpreter, len);
// Inform the optimizer that the bytecode cannot be EOF to remove a bounds check.
assume!(!interpreter.contract.bytecode.is_eof());
let source = interpreter.contract.bytecode.original_byte_slice().to_vec();
copy_to_memory(interpreter, memory_offset, code_offset, len, &source);
let source = interpreter.contract.bytecode.original_byte_slice().as_ptr();
copy_to_memory(interpreter, memory_offset, code_offset, len, source);
}

pub fn calldataload<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
Expand Down Expand Up @@ -83,8 +83,8 @@ pub fn calldatacopy<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut
pop!(interpreter, memory_offset, data_offset, len);
let len = as_usize_or_fail!(interpreter, len);
let data_offset = as_usize_saturated!(data_offset);
let source = interpreter.contract.input.to_vec();
copy_to_memory(interpreter, memory_offset, data_offset, len, &source);
let source = interpreter.contract.input.as_ptr();
copy_to_memory(interpreter, memory_offset, data_offset, len, source);
}

/// EIP-211: New opcodes: RETURNDATASIZE and RETURNDATACOPY
Expand Down Expand Up @@ -113,8 +113,8 @@ pub fn returndatacopy<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interprete
return;
}

let source = interpreter.return_data_buffer.to_vec();
copy_to_memory(interpreter, memory_offset, data_offset, len, &source);
let source = interpreter.return_data_buffer.as_ptr();
copy_to_memory(interpreter, memory_offset, data_offset, len, source);
}

/// Part of EOF `<https://eips.ethereum.org/EIPS/eip-7069>`.
Expand Down Expand Up @@ -145,7 +145,7 @@ fn copy_to_memory(
memory_offset: U256,
data_offset: usize,
len: usize,
source: &[u8],
source: *const u8,
Copy link
Copy Markdown
Collaborator

@DaniPopes DaniPopes Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

source's length is not len, this should be *const [u8]. This is why tests are failing

) {
Copy link
Copy Markdown
Member

@rakita rakita Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would return a Option<usize> here that represents memory_offset.
https://github.com/bluealloy/revm/pull/1655/files#r1694998333

Without return function silently fails, and can work only if it is in the last line of instruction.

gas_or_fail!(interpreter, gas::verylowcopy_cost(len as u64));
if len == 0 {
Expand All @@ -155,9 +155,11 @@ fn copy_to_memory(
resize_memory!(interpreter, memory_offset, len);

// Note: this can't panic because we resized memory to fit.
interpreter
.shared_memory
.set_data(memory_offset, data_offset, len, source);
unsafe {
interpreter
.shared_memory
.set_data(memory_offset, data_offset, len, std::slice::from_raw_parts(source, len));
}
}

pub fn gas<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
Expand Down