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

perf: infer when to use local stack #64

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion crates/revmc/src/bytecode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,11 +545,17 @@ impl<'a> Bytecode<'a> {
);
}

/// Returns the EOF container, if any.
#[inline]
pub fn eof(&self) -> Option<&Eof> {
self.eof.as_deref()
}

/// Returns the `Eof` container, panicking if it is not set.
#[track_caller]
#[inline]
pub(crate) fn expect_eof(&self) -> &Eof {
self.eof.as_deref().expect("EOF container not set")
self.eof().expect("EOF container not set")
}

/// Returns the name for a basic block.
Expand Down
12 changes: 3 additions & 9 deletions crates/revmc/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ use crate::{Backend, Builder, Bytecode, EvmCompilerFn, EvmContext, EvmStack, Res
use revm_interpreter::{Contract, Gas};
use revm_primitives::{Bytes, Env, Eof, SpecId, EOF_MAGIC_BYTES};
use revmc_backend::{
eyre::{ensure, eyre},
Attribute, FunctionAttributeLocation, Linkage, OptimizationLevel,
eyre::ensure, Attribute, FunctionAttributeLocation, Linkage, OptimizationLevel,
};
use revmc_builtins::Builtins;
use revmc_context::RawEvmCompilerFn;
Expand Down Expand Up @@ -165,7 +164,7 @@ impl<B: Backend> EvmCompiler<B> {
self.config.validate_eof = yes;
}

/// Sets whether to allocate the stack locally.
/// Forces the stack to be allocated inside of the function, instead of passed as an argument.
///
/// If this is set to `true`, the stack pointer argument will be ignored and the stack will be
/// allocated in the function.
Expand Down Expand Up @@ -359,12 +358,7 @@ impl<B: Backend> EvmCompiler<B> {
if !self.config.validate_eof {
return Ok(());
}
revm_interpreter::analysis::validate_eof_inner(eof, None).map_err(|e| match e {
revm_interpreter::analysis::EofError::Decode(e) => e.into(),
revm_interpreter::analysis::EofError::Validation(e) => {
eyre!("validation error: {e:?}")
}
})
revm_interpreter::analysis::validate_eof_inner(eof, None).map_err(Into::into)
}

#[instrument(name = "translate", level = "debug", skip_all)]
Expand Down
17 changes: 11 additions & 6 deletions crates/revmc/src/compiler/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,17 @@ impl<'a, B: Backend> FunctionCx<'a, B> {
Pointer::new_address(i64_type, bcx.gep(i8_type, gas_ptr, &[offset], name))
};

// We store the stack length if requested or necessary due to the bytecode.
let stack_length_observable = config.inspect_stack_length || bytecode.may_suspend();
let local_stack = config.local_stack || !stack_length_observable;
let sp_arg = bcx.fn_param(1);
let stack = if config.local_stack {
bcx.new_stack_slot(word_type, "stack.addr")
let stack = if local_stack {
let max_size = bytecode
.eof()
.map(|eof| eof.body.types_section.iter().map(|s| s.max_stack_size).max().unwrap())
.unwrap_or(STACK_CAP as _);
let size = max_size.next_power_of_two().min(STACK_CAP as _);
bcx.new_stack_slot(bcx.type_array(word_type, size as u32), "stack.addr")
} else {
Pointer::new_address(word_type, sp_arg)
};
Expand Down Expand Up @@ -281,9 +289,6 @@ impl<'a, B: Backend> FunctionCx<'a, B> {
builtins,
};

// We store the stack length if requested or necessary due to the bytecode.
let stack_length_observable = config.inspect_stack_length || bytecode.may_suspend();

// Add debug assertions for the parameters.
if config.debug_assertions {
fx.pointer_panic_with_bool(
Expand All @@ -293,7 +298,7 @@ impl<'a, B: Backend> FunctionCx<'a, B> {
"gas metering is enabled",
);
fx.pointer_panic_with_bool(
!config.local_stack,
!local_stack,
sp_arg,
"stack pointer",
"local stack is disabled",
Expand Down
8 changes: 6 additions & 2 deletions tests/codegen/address_mask.evm
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ ADDRESS
PUSH20 0xffffffffffffffffffffffffffffffffffffffff
AND

PUSH0 MSTORE
PUSH1 0x20 PUSH0 RETURN

; CHECK: load i160
; CHECK: [[ADDR:%.*]] = zext i160 {{.*}} to i256
; CHECK: zext i160 {{.*}} to i256
; CHECK-NOT: and
; CHECK: store i256 [[ADDR]]
; CHECK: call {{.*}}mstore
; CHECK: call {{.*}}return
; CHECK: ret i8
8 changes: 6 additions & 2 deletions tests/codegen/non_zero.evm
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ CALLVALUE
ISZERO
ISZERO

PUSH0 MSTORE
PUSH1 0x20 PUSH0 RETURN

; CHECK: [[VALUE:%.*value.*]] = load i256
; CHECK-NEXT: [[NON_ZERO:%.*]] = icmp ne i256 [[VALUE]], 0
; CHECK-NEXT: [[EXTENDED:%.*]] = zext i1 [[NON_ZERO]] to i256
; CHECK-NEXT: store i256 [[EXTENDED]]
; CHECK-NEXT: br label %return
; CHECK: call {{.*}}mstore
; CHECK: call {{.*}}return
; CHECK: ret i8
Loading