From 355c250f37cf0977ef2776b1aae2cb2e87c9da3d Mon Sep 17 00:00:00 2001 From: Pedro Fontana Date: Mon, 21 Oct 2024 13:33:58 -0300 Subject: [PATCH] Revert builtin cost rework (#874) * Revert "fix aot contract executor not passing builtinstats (#849)" This reverts commit aff451c4821fb593eced61a418987a95d3410524. * Revert "Builtin costs rework (#837)" This reverts commit f862ec3533a7af79909f4c3e19598da2e537cd91. --------- Co-authored-by: pefontana --- programs/benches/factorial_2M.c | 5 -- programs/benches/fib_2M.c | 5 -- programs/benches/logistic_map.c | 5 -- src/compiler.rs | 28 +------ src/error.rs | 3 - src/executor.rs | 69 +++++----------- src/executor/aot.rs | 12 --- src/executor/contract.rs | 79 +++---------------- src/executor/jit.rs | 13 --- src/libfuncs/gas.rs | 135 ++++---------------------------- src/metadata/gas.rs | 15 ++-- src/types.rs | 10 +-- src/types/builtin_costs.rs | 8 +- src/utils.rs | 44 ----------- src/values.rs | 4 +- tests/common.rs | 16 ++-- 16 files changed, 70 insertions(+), 381 deletions(-) diff --git a/programs/benches/factorial_2M.c b/programs/benches/factorial_2M.c index 9beefef5f..7db0cfcc6 100644 --- a/programs/benches/factorial_2M.c +++ b/programs/benches/factorial_2M.c @@ -16,7 +16,6 @@ typedef struct factorial_return_values } result; } factorial_return_values_t; -extern uint64_t* builtin_costs; static void run_bench(factorial_return_values_t*, uint64_t) __attribute__((weakref("_mlir_ciface_factorial_2M::factorial_2M::main(f1)"))); @@ -26,10 +25,6 @@ int main() { factorial_return_values_t return_values; - uint64_t BuiltinCosts[7] = {1, 4050, 583, 4085, 491, 230, 604}; - - builtin_costs = &BuiltinCosts[0]; - run_bench(&return_values, 0); assert(return_values.result.discriminant == 0); diff --git a/programs/benches/fib_2M.c b/programs/benches/fib_2M.c index 72a420a86..fecb87cb3 100644 --- a/programs/benches/fib_2M.c +++ b/programs/benches/fib_2M.c @@ -16,7 +16,6 @@ typedef struct fib_return_values } result; } fib_return_values_t; -extern uint64_t* builtin_costs; static void run_bench(fib_return_values_t *, uint64_t) __attribute__((weakref("_mlir_ciface_fib_2M::fib_2M::main(f1)"))); @@ -24,10 +23,6 @@ static void run_bench(fib_return_values_t *, uint64_t) int main() { - uint64_t BuiltinCosts[7] = {1, 4050, 583, 4085, 491, 230, 604}; - - builtin_costs = &BuiltinCosts[0]; - fib_return_values_t return_values; run_bench(&return_values, 0); diff --git a/programs/benches/logistic_map.c b/programs/benches/logistic_map.c index 483def4c4..1294dcdbf 100644 --- a/programs/benches/logistic_map.c +++ b/programs/benches/logistic_map.c @@ -16,7 +16,6 @@ typedef struct map_return_values } result; } map_return_values_t; -extern uint64_t* builtin_costs; static void run_bench(map_return_values_t *, uint64_t) __attribute__((weakref("_mlir_ciface_logistic_map::logistic_map::main(f2)"))); @@ -24,10 +23,6 @@ static void run_bench(map_return_values_t *, uint64_t) int main() { - uint64_t BuiltinCosts[7] = {1, 4050, 583, 4085, 491, 230, 604}; - - builtin_costs = &BuiltinCosts[0]; - map_return_values_t return_values; run_bench(&return_values, 0); diff --git a/src/compiler.rs b/src/compiler.rs index 85c4e2059..f0efa38d4 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -74,7 +74,7 @@ use melior::{ arith::CmpiPredicate, cf, func, index, llvm::{self, LoadStoreOptions}, - memref, ods, + memref, }, ir::{ attribute::{ @@ -135,30 +135,6 @@ pub fn compile( } } - { - // Add the builtin_costs global. - // We always add it because symbol look up otherwise can panic. - let region = Region::new(); - let location = Location::unknown(context); - let block = region.append_block(Block::new(&[])); - let value = block.append_op_result( - ods::llvm::mlir_zero(context, llvm::r#type::pointer(context, 0), location).into(), - )?; - block.append_operation(melior::dialect::llvm::r#return(Some(value), location)); - - module.body().append_operation( - ods::llvm::mlir_global( - context, - region, - TypeAttribute::new(llvm::r#type::pointer(context, 0)), - StringAttribute::new(context, "builtin_costs"), - Attribute::parse(context, "#llvm.linkage").unwrap(), - location, - ) - .into(), - ); - } - // Sierra programs have the following structure: // 1. Type declarations, one per line. // 2. Libfunc declarations, one per line. @@ -470,7 +446,7 @@ fn compile_func( initial_state, |statement_idx, mut state| { if let Some(gas_metadata) = metadata.get::() { - let gas_cost = gas_metadata.get_gas_costs_for_statement(statement_idx); + let gas_cost = gas_metadata.get_gas_cost_for_statement(statement_idx); metadata.remove::(); metadata.insert(GasCost(gas_cost)); } diff --git a/src/error.rs b/src/error.rs index 2cc503d07..c58b2fa57 100644 --- a/src/error.rs +++ b/src/error.rs @@ -69,9 +69,6 @@ pub enum Error { #[error("integer conversion failed")] IntegerConversion, - #[error("missing BuiltinCosts global symbol, should never happen, this is a bug")] - MissingBuiltinCostsSymbol, - #[error(transparent)] IoError(#[from] std::io::Error), diff --git a/src/executor.rs b/src/executor.rs index f88e87735..13a570ac6 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -10,7 +10,7 @@ use crate::{ execution_result::{BuiltinStats, ExecutionResult}, starknet::{handler::StarknetSyscallHandlerCallbacks, StarknetSyscallHandler}, types::TypeBuilder, - utils::{libc_free, BuiltinCosts, RangeExt}, + utils::{libc_free, RangeExt}, values::Value, }; use bumpalo::Bump; @@ -69,7 +69,6 @@ extern "C" { fn invoke_dynamic( registry: &ProgramRegistry, function_ptr: *const c_void, - builtin_costs_ptr: Option<*mut c_void>, function_signature: &FunctionSignature, args: &[Value], gas: u128, @@ -142,15 +141,6 @@ fn invoke_dynamic( previous_syscall_handler }); - // Order matters, for the libfunc impl - let builtin_costs: [u64; 7] = BuiltinCosts::default().into(); - - if let Some(builtin_costs_ptr) = builtin_costs_ptr { - unsafe { - *builtin_costs_ptr.cast() = builtin_costs.as_ptr(); - } - } - // Generate argument list. let mut iter = args.iter(); for item in function_signature.param_types.iter().filter_map(|type_id| { @@ -176,13 +166,6 @@ fn invoke_dynamic( (syscall_handler as *mut StarknetSyscallHandlerCallbacks<_>) .to_bytes(&mut invoke_data)?; } - CoreTypeConcrete::BuiltinCosts(_) => { - if let Some(builtin_costs_ptr) = builtin_costs_ptr { - builtin_costs_ptr.to_bytes(&mut invoke_data)?; - } else { - (builtin_costs.as_ptr()).to_bytes(&mut invoke_data)?; - } - } type_info if type_info.is_builtin() => 0u64.to_bytes(&mut invoke_data)?, type_info => ValueWithInfoWrapper { value: iter.next().unwrap(), @@ -266,38 +249,26 @@ fn invoke_dynamic( }, _ if type_info.is_builtin() => { if !type_info.is_zst(registry)? { - if let CoreTypeConcrete::BuiltinCosts(_) = type_info { - // todo: should we use this value? - let _value = match &mut return_ptr { - Some(return_ptr) => unsafe { *read_value::<*mut u64>(return_ptr) }, - None => ret_registers[0] as *mut u64, - }; - } else { - let value = match &mut return_ptr { - Some(return_ptr) => unsafe { *read_value::(return_ptr) }, - None => ret_registers[0], - } as usize; - - match type_info { - CoreTypeConcrete::Bitwise(_) => builtin_stats.bitwise = value, - CoreTypeConcrete::EcOp(_) => builtin_stats.ec_op = value, - CoreTypeConcrete::RangeCheck(_) => builtin_stats.range_check = value, - CoreTypeConcrete::Pedersen(_) => builtin_stats.pedersen = value, - CoreTypeConcrete::Poseidon(_) => builtin_stats.poseidon = value, - CoreTypeConcrete::SegmentArena(_) => { - builtin_stats.segment_arena = value - } - CoreTypeConcrete::RangeCheck96(_) => { - builtin_stats.range_check_96 = value - } - CoreTypeConcrete::Circuit(CircuitTypeConcrete::AddMod(_)) => { - builtin_stats.circuit_add = value - } - CoreTypeConcrete::Circuit(CircuitTypeConcrete::MulMod(_)) => { - builtin_stats.circuit_mul = value - } - _ => unreachable!("{type_id:?}"), + let value = match &mut return_ptr { + Some(return_ptr) => unsafe { *read_value::(return_ptr) }, + None => ret_registers[0], + } as usize; + + match type_info { + CoreTypeConcrete::Bitwise(_) => builtin_stats.bitwise = value, + CoreTypeConcrete::EcOp(_) => builtin_stats.ec_op = value, + CoreTypeConcrete::RangeCheck(_) => builtin_stats.range_check = value, + CoreTypeConcrete::Pedersen(_) => builtin_stats.pedersen = value, + CoreTypeConcrete::Poseidon(_) => builtin_stats.poseidon = value, + CoreTypeConcrete::SegmentArena(_) => builtin_stats.segment_arena = value, + CoreTypeConcrete::RangeCheck96(_) => builtin_stats.range_check_96 = value, + CoreTypeConcrete::Circuit(CircuitTypeConcrete::AddMod(_)) => { + builtin_stats.circuit_add = value + } + CoreTypeConcrete::Circuit(CircuitTypeConcrete::MulMod(_)) => { + builtin_stats.circuit_mul = value } + _ => unreachable!("{type_id:?}"), } } } diff --git a/src/executor/aot.rs b/src/executor/aot.rs index ebaed4c3e..cac2122e2 100644 --- a/src/executor/aot.rs +++ b/src/executor/aot.rs @@ -81,7 +81,6 @@ impl AotNativeExecutor { super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id), args, available_gas, @@ -104,7 +103,6 @@ impl AotNativeExecutor { super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id), args, available_gas, @@ -127,7 +125,6 @@ impl AotNativeExecutor { ContractExecutionResult::from_execution_result(super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id), &[Value::Struct { fields: vec![Value::Array( @@ -155,15 +152,6 @@ impl AotNativeExecutor { } } - pub fn find_symbol_ptr(&self, name: &str) -> Option<*mut c_void> { - unsafe { - self.library - .get::<*mut ()>(name.as_bytes()) - .ok() - .map(|x| x.into_raw().into_raw()) - } - } - fn extract_signature(&self, function_id: &FunctionId) -> &FunctionSignature { &self.registry.get_function(function_id).unwrap().signature } diff --git a/src/executor/contract.rs b/src/executor/contract.rs index 31f9f0b01..14c4130f2 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -34,7 +34,7 @@ use crate::{ arch::AbiArgument, context::NativeContext, - error::{Error, Result}, + error::Result, execution_result::{BuiltinStats, ContractExecutionResult}, executor::invoke_trampoline, module::NativeModule, @@ -42,7 +42,6 @@ use crate::{ types::TypeBuilder, utils::{ decode_error_message, generate_function_name, get_integer_layout, libc_free, libc_malloc, - BuiltinCosts, }, OptLevel, }; @@ -77,27 +76,16 @@ pub struct AotContractExecutor { library: Arc, path: PathBuf, is_temp_path: bool, - contract_info: ContractInfo, + entry_points_info: BTreeMap, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub struct ContractInfo { - pub version: ContractInfoVersion, - pub entry_points: BTreeMap, +struct EntryPointInfo { + builtins: Vec, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub enum ContractInfoVersion { - Version0, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub struct EntryPointInfo { - pub builtins: Vec, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub enum BuiltinType { +enum BuiltinType { Bitwise, EcOp, RangeCheck, @@ -109,7 +97,6 @@ pub enum BuiltinType { CircuitMul, Gas, System, - BuiltinCosts, } impl AotContractExecutor { @@ -147,9 +134,6 @@ impl AotContractExecutor { CoreTypeConcrete::RangeCheck(_) => builtins.push(BuiltinType::RangeCheck), CoreTypeConcrete::Pedersen(_) => builtins.push(BuiltinType::Pedersen), CoreTypeConcrete::Poseidon(_) => builtins.push(BuiltinType::Poseidon), - CoreTypeConcrete::BuiltinCosts(_) => { - builtins.push(BuiltinType::BuiltinCosts) - } CoreTypeConcrete::SegmentArena(_) => { builtins.push(BuiltinType::SegmentArena) } @@ -188,10 +172,7 @@ impl AotContractExecutor { library: Arc::new(unsafe { Library::new(&library_path)? }), path: library_path, is_temp_path: true, - contract_info: ContractInfo { - version: ContractInfoVersion::Version0, - entry_points: infos, - }, + entry_points_info: infos, }) } @@ -200,9 +181,9 @@ impl AotContractExecutor { let to = to.as_ref(); std::fs::copy(&self.path, to)?; - let contract_info = serde_json::to_string(&self.contract_info)?; + let info = serde_json::to_string(&self.entry_points_info)?; let path = to.with_extension("json"); - std::fs::write(path, contract_info)?; + std::fs::write(path, info)?; self.path = to.to_path_buf(); self.is_temp_path = false; @@ -213,12 +194,12 @@ impl AotContractExecutor { /// Load the executor from an already compiled library with the additional info json file. pub fn load(library_path: &Path) -> Result { let info_str = std::fs::read_to_string(library_path.with_extension("json"))?; - let contract_info: ContractInfo = serde_json::from_str(&info_str)?; + let info: BTreeMap = serde_json::from_str(&info_str)?; Ok(Self { library: Arc::new(unsafe { Library::new(library_path)? }), path: library_path.to_path_buf(), is_temp_path: false, - contract_info, + entry_points_info: info, }) } @@ -228,30 +209,16 @@ impl AotContractExecutor { function_id: &FunctionId, args: &[Felt], gas: Option, - builtin_costs: Option, mut syscall_handler: impl StarknetSyscallHandler, ) -> Result { let arena = Bump::new(); let mut invoke_data = Vec::::new(); let function_ptr = self.find_function_ptr(function_id, true)?; - let builtin_costs_ptr = self - .find_symbol_ptr("builtin_costs") - .ok_or_else(|| Error::MissingBuiltinCostsSymbol)?; - - let builtin_costs = builtin_costs.unwrap_or_default(); - let builtin_costs: [u64; 7] = builtin_costs.into(); - - unsafe { - *builtin_costs_ptr.cast() = builtin_costs.as_ptr(); - } // it can vary from contract to contract thats why we need to store/ load it. // substract 2, which are the gas and syscall builtin - let num_builtins = self.contract_info.entry_points[&function_id.id] - .builtins - .len() - - 2; + let num_builtins = self.entry_points_info[&function_id.id].builtins.len() - 2; // There is always a return ptr because contracts always return more than 1 thing (builtin counters, syscall, enum) let return_ptr = arena.alloc_layout(unsafe { @@ -264,15 +231,12 @@ impl AotContractExecutor { let mut syscall_handler = StarknetSyscallHandlerCallbacks::new(&mut syscall_handler); - for b in &self.contract_info.entry_points[&function_id.id].builtins { + for b in &self.entry_points_info[&function_id.id].builtins { match b { BuiltinType::Gas => { let gas = gas.unwrap_or(0); gas.to_bytes(&mut invoke_data)?; } - BuiltinType::BuiltinCosts => { - builtin_costs_ptr.to_bytes(&mut invoke_data)?; - } BuiltinType::System => { (&mut syscall_handler as *mut StarknetSyscallHandlerCallbacks<_>) .to_bytes(&mut invoke_data)?; @@ -347,7 +311,7 @@ impl AotContractExecutor { let return_ptr = &mut return_ptr.cast(); - for b in &self.contract_info.entry_points[&function_id.id].builtins { + for b in &self.entry_points_info[&function_id.id].builtins { match b { BuiltinType::Gas => { remaining_gas = unsafe { *read_value::(return_ptr) }; @@ -356,11 +320,6 @@ impl AotContractExecutor { let ptr = return_ptr.cast::<*mut ()>(); *return_ptr = unsafe { NonNull::new_unchecked(ptr.as_ptr().add(1)).cast() }; } - BuiltinType::BuiltinCosts => { - let ptr = return_ptr.cast::<*mut ()>(); - *return_ptr = unsafe { NonNull::new_unchecked(ptr.as_ptr().add(1)).cast() }; - // ptr holds the builtin costs, but they dont change, so its of no use, but we read to advance the ptr. - } x => { let value = unsafe { *read_value::(return_ptr) } as usize; @@ -376,7 +335,6 @@ impl AotContractExecutor { BuiltinType::CircuitMul => builtin_stats.circuit_mul = value, BuiltinType::Gas => {} BuiltinType::System => {} - BuiltinType::BuiltinCosts => {} } } } @@ -478,15 +436,6 @@ impl AotContractExecutor { .into_raw() }) } - - pub fn find_symbol_ptr(&self, name: &str) -> Option<*mut c_void> { - unsafe { - self.library - .get::<*mut ()>(name.as_bytes()) - .ok() - .map(|x| x.into_raw().into_raw()) - } - } } impl Drop for AotContractExecutor { @@ -570,7 +519,6 @@ mod tests { entrypoint_function_id, &[2.into()], Some(u64::MAX as u128), - None, &mut StubSyscallHandler::default(), ) .unwrap(); @@ -596,7 +544,6 @@ mod tests { entrypoint_function_id, &[], Some(u64::MAX as u128), - None, &mut StubSyscallHandler::default(), ) .unwrap(); diff --git a/src/executor/jit.rs b/src/executor/jit.rs index 4b50ce113..55bfafcbe 100644 --- a/src/executor/jit.rs +++ b/src/executor/jit.rs @@ -79,7 +79,6 @@ impl<'m> JitNativeExecutor<'m> { super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id).unwrap(), args, available_gas, @@ -103,7 +102,6 @@ impl<'m> JitNativeExecutor<'m> { super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id).unwrap(), args, available_gas, @@ -126,7 +124,6 @@ impl<'m> JitNativeExecutor<'m> { ContractExecutionResult::from_execution_result(super::invoke_dynamic( &self.registry, self.find_function_ptr(function_id), - self.find_symbol_ptr("builtin_costs"), self.extract_signature(function_id).unwrap(), &[Value::Struct { fields: vec![Value::Array( @@ -148,16 +145,6 @@ impl<'m> JitNativeExecutor<'m> { self.engine.lookup(&function_name) as *mut c_void } - pub fn find_symbol_ptr(&self, name: &str) -> Option<*mut c_void> { - let ptr = self.engine.lookup(name) as *mut c_void; - - if ptr.is_null() { - None - } else { - Some(ptr) - } - } - fn extract_signature(&self, function_id: &FunctionId) -> Option<&FunctionSignature> { self.program_registry() .get_function(function_id) diff --git a/src/libfuncs/gas.rs b/src/libfuncs/gas.rs index b7eabecc9..474fe7443 100644 --- a/src/libfuncs/gas.rs +++ b/src/libfuncs/gas.rs @@ -9,7 +9,7 @@ use crate::{ use cairo_lang_sierra::{ extensions::{ core::{CoreLibfunc, CoreType}, - gas::{CostTokenType, GasConcreteLibfunc}, + gas::GasConcreteLibfunc, lib_func::SignatureOnlyConcreteLibfunc, ConcreteLibfunc, }, @@ -18,10 +18,9 @@ use cairo_lang_sierra::{ use melior::{ dialect::{ arith::{self, CmpiPredicate}, - llvm::{self, r#type::pointer}, - ods, + llvm, ods, }, - ir::{attribute::FlatSymbolRefAttribute, r#type::IntegerType, Block, Location}, + ir::{r#type::IntegerType, Block, Location}, Context, }; @@ -84,74 +83,22 @@ pub fn build_withdraw_gas<'ctx, 'this>( super::increment_builtin_counter(context, entry, location, entry.argument(0)?.into())?; let current_gas = entry.argument(1)?.into(); - let gas_cost = metadata - .get::() - .expect("builtin_withdraw_gas should always have a gas cost"); + let cost = metadata.get::().and_then(|x| x.0); let u128_type: melior::ir::Type = IntegerType::new(context, 128).into(); - let u64_type: melior::ir::Type = IntegerType::new(context, 64).into(); - - // Get the ptr to the global, holding a ptr to the list. - let builtin_ptr_ptr = entry.append_op_result( - ods::llvm::mlir_addressof( - context, - pointer(context, 0), - FlatSymbolRefAttribute::new(context, "builtin_costs"), - location, - ) - .into(), - )?; - - let builtin_ptr = entry.load(context, location, builtin_ptr_ptr, pointer(context, 0))?; - - let mut final_gas_cost = entry.const_int_from_type(context, location, 0, u128_type)?; - - for (cost, token_type) in &gas_cost.0 { - if *cost == 0 { - continue; - } - - let token_type_index = match token_type { - CostTokenType::Const => 0, - CostTokenType::Pedersen => 1, - CostTokenType::Bitwise => 2, - CostTokenType::EcOp => 3, - CostTokenType::Poseidon => 4, - CostTokenType::AddMod => 5, - CostTokenType::MulMod => 6, - _ => unreachable!(), - }; - - let gas_cost_val = entry.const_int_from_type(context, location, *cost, u128_type)?; - let token_type_index_val = - entry.const_int_from_type(context, location, token_type_index, u64_type)?; - - let cost_value_ptr = entry.append_op_result(llvm::get_element_ptr_dynamic( - context, - builtin_ptr, - &[token_type_index_val], - u64_type, - pointer(context, 0), - location, - ))?; - let cost_value = entry.load(context, location, cost_value_ptr, u64_type)?; - let cost_value = entry.append_op_result(arith::extui(cost_value, u128_type, location))?; - let total_gas_cost_val = - entry.append_op_result(arith::muli(gas_cost_val, cost_value, location))?; - final_gas_cost = - entry.append_op_result(arith::addi(final_gas_cost, total_gas_cost_val, location))?; - } + let gas_cost_val = + entry.const_int_from_type(context, location, cost.unwrap_or(0), u128_type)?; let is_enough = entry.append_op_result(arith::cmpi( context, CmpiPredicate::Uge, current_gas, - final_gas_cost, + gas_cost_val, location, ))?; let resulting_gas = entry.append_op_result( - ods::llvm::intr_usub_sat(context, current_gas, final_gas_cost, location).into(), + ods::llvm::intr_usub_sat(context, current_gas, gas_cost_val, location).into(), )?; entry.append_operation(helper.cond_br( @@ -178,63 +125,23 @@ pub fn build_builtin_withdraw_gas<'ctx, 'this>( let range_check = super::increment_builtin_counter(context, entry, location, entry.argument(0)?.into())?; let current_gas = entry.argument(1)?.into(); - let builtin_ptr = entry.argument(2)?.into(); - let gas_cost = metadata - .get::() - .expect("builtin_withdraw_gas should always have a gas cost"); + let cost = metadata.get::().and_then(|x| x.0); let u128_type: melior::ir::Type = IntegerType::new(context, 128).into(); - let u64_type: melior::ir::Type = IntegerType::new(context, 64).into(); - - let mut final_gas_cost = entry.const_int_from_type(context, location, 0, u128_type)?; - - for (cost, token_type) in &gas_cost.0 { - if *cost == 0 { - continue; - } - - let token_type_index = match token_type { - CostTokenType::Const => 0, - CostTokenType::Pedersen => 1, - CostTokenType::Bitwise => 2, - CostTokenType::EcOp => 3, - CostTokenType::Poseidon => 4, - CostTokenType::AddMod => 5, - CostTokenType::MulMod => 6, - _ => unreachable!(), - }; - - let gas_cost_val = entry.const_int_from_type(context, location, *cost, u128_type)?; - let token_type_index_val = - entry.const_int_from_type(context, location, token_type_index, u64_type)?; - - let cost_value_ptr = entry.append_op_result(llvm::get_element_ptr_dynamic( - context, - builtin_ptr, - &[token_type_index_val], - u64_type, - pointer(context, 0), - location, - ))?; - let cost_value = entry.load(context, location, cost_value_ptr, u64_type)?; - let cost_value = entry.append_op_result(arith::extui(cost_value, u128_type, location))?; - let total_gas_cost_val = - entry.append_op_result(arith::muli(gas_cost_val, cost_value, location))?; - final_gas_cost = - entry.append_op_result(arith::addi(final_gas_cost, total_gas_cost_val, location))?; - } + let gas_cost_val = + entry.const_int_from_type(context, location, cost.unwrap_or(0), u128_type)?; let is_enough = entry.append_op_result(arith::cmpi( context, CmpiPredicate::Uge, current_gas, - final_gas_cost, + gas_cost_val, location, ))?; let resulting_gas = entry.append_op_result( - ods::llvm::intr_usub_sat(context, current_gas, final_gas_cost, location).into(), + ods::llvm::intr_usub_sat(context, current_gas, gas_cost_val, location).into(), )?; entry.append_operation(helper.cond_br( @@ -266,20 +173,10 @@ pub fn build_get_builtin_costs<'ctx, 'this>( &info.branch_signatures()[0].vars[0].ty, )?; - // Get the ptr to the global, holding a ptr to the list. - let builtin_ptr_ptr = entry.append_op_result( - ods::llvm::mlir_addressof( - context, - pointer(context, 0), - FlatSymbolRefAttribute::new(context, "builtin_costs"), - location, - ) - .into(), - )?; - - let builtin_ptr = entry.load(context, location, builtin_ptr_ptr, builtin_costs_ty)?; + // TODO: Implement libfunc. + let op0 = entry.append_op_result(llvm::undef(builtin_costs_ty, location))?; - entry.append_operation(helper.br(0, &[builtin_ptr], location)); + entry.append_operation(helper.br(0, &[op0], location)); Ok(()) } diff --git a/src/metadata/gas.rs b/src/metadata/gas.rs index 9bb2ac34b..9378c5895 100644 --- a/src/metadata/gas.rs +++ b/src/metadata/gas.rs @@ -20,9 +20,8 @@ pub struct GasMetadata { pub gas_info: GasInfo, } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -// Cost, token type (index into builtin costs). -pub struct GasCost(pub Vec<(u128, CostTokenType)>); +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct GasCost(pub Option); /// Configuration for metadata computation. #[derive(Debug, Clone)] @@ -103,18 +102,16 @@ impl GasMetadata { ) } - pub fn get_gas_costs_for_statement(&self, idx: StatementIdx) -> Vec<(u128, CostTokenType)> { - let mut costs = Vec::new(); + pub fn get_gas_cost_for_statement(&self, idx: StatementIdx) -> Option { + let mut cost = None; for cost_type in CostTokenType::iter_casm_tokens() { if let Some(amount) = self.get_gas_cost_for_statement_and_cost_token_type(idx, *cost_type) { - if amount > 0 { - costs.push((amount, *cost_type)); - } + *cost.get_or_insert(0) += amount * token_gas_cost(*cost_type) as u128; } } - costs + cost } pub fn get_gas_cost_for_statement_and_cost_token_type( diff --git a/src/types.rs b/src/types.rs index 6d0d6314e..bf1dbb8bf 100644 --- a/src/types.rs +++ b/src/types.rs @@ -549,12 +549,10 @@ impl TypeBuilder for CoreTypeConcrete { | CoreTypeConcrete::Poseidon(_) | CoreTypeConcrete::RangeCheck96(_) | CoreTypeConcrete::SegmentArena(_) => false, - - // A ptr to a list of costs. - CoreTypeConcrete::BuiltinCosts(_) => false, - // Other builtins: - CoreTypeConcrete::Uint128MulGuarantee(_) | CoreTypeConcrete::Coupon(_) => true, + CoreTypeConcrete::BuiltinCosts(_) + | CoreTypeConcrete::Uint128MulGuarantee(_) + | CoreTypeConcrete::Coupon(_) => true, // Normal types: CoreTypeConcrete::Array(_) @@ -636,7 +634,7 @@ impl TypeBuilder for CoreTypeConcrete { CoreTypeConcrete::EcState(_) => layout_repeat(&get_integer_layout(252), 4)?.0, CoreTypeConcrete::Felt252(_) => get_integer_layout(252), CoreTypeConcrete::GasBuiltin(_) => get_integer_layout(128), - CoreTypeConcrete::BuiltinCosts(_) => Layout::new::<*const ()>(), + CoreTypeConcrete::BuiltinCosts(_) => Layout::new::<()>(), CoreTypeConcrete::Uint8(_) => get_integer_layout(8), CoreTypeConcrete::Uint16(_) => get_integer_layout(16), CoreTypeConcrete::Uint32(_) => get_integer_layout(32), diff --git a/src/types/builtin_costs.rs b/src/types/builtin_costs.rs index 7fc2e978c..b7a202dce 100644 --- a/src/types/builtin_costs.rs +++ b/src/types/builtin_costs.rs @@ -1,7 +1,4 @@ //! # Builtin costs type -//! -//! A ptr to a list of u64, this list will not change at runtime in size and thus we only really need to store the pointer, -//! it can be allocated on the stack on rust side and passed. use super::WithSelf; use crate::{error::Result, metadata::MetadataStorage}; @@ -14,7 +11,7 @@ use cairo_lang_sierra::{ }; use melior::{ dialect::llvm, - ir::{Module, Type}, + ir::{r#type::IntegerType, Module, Type}, Context, }; @@ -28,6 +25,5 @@ pub fn build<'ctx>( _metadata: &mut MetadataStorage, _info: WithSelf, ) -> Result> { - // A ptr to a list of u64 - Ok(llvm::r#type::pointer(context, 0)) + Ok(llvm::r#type::array(IntegerType::new(context, 8).into(), 0)) } diff --git a/src/utils.rs b/src/utils.rs index 090b03292..2c5fc73d9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -5,9 +5,7 @@ pub(crate) use self::{ }; use crate::{metadata::MetadataStorage, OptLevel}; use cairo_lang_compiler::CompilerConfig; -use cairo_lang_runner::token_gas_cost; use cairo_lang_sierra::{ - extensions::gas::CostTokenType, ids::FunctionId, program::{GenFunction, Program, StatementIdx}, }; @@ -17,7 +15,6 @@ use melior::{ Context, Error, ExecutionEngine, }; use num_bigint::{BigInt, BigUint, Sign}; -use serde::{Deserialize, Serialize}; use starknet_types_core::felt::Felt; use std::sync::LazyLock; use std::{ @@ -52,47 +49,6 @@ pub static HALF_PRIME: LazyLock = LazyLock::new(|| { .unwrap() }); -#[derive(Debug, Clone, Copy, Deserialize, Serialize)] -pub struct BuiltinCosts { - pub r#const: u64, - pub pedersen: u64, - pub bitwise: u64, - pub ecop: u64, - pub poseidon: u64, - pub add_mod: u64, - pub mul_mod: u64, -} - -impl From for [u64; 7] { - // Order matters, for the libfunc impl - // https://github.com/starkware-libs/sequencer/blob/1b7252f8a30244d39614d7666aa113b81291808e/crates/blockifier/src/execution/entry_point_execution.rs#L208 - fn from(value: BuiltinCosts) -> Self { - [ - value.r#const, - value.pedersen, - value.bitwise, - value.ecop, - value.poseidon, - value.add_mod, - value.mul_mod, - ] - } -} - -impl Default for BuiltinCosts { - fn default() -> Self { - Self { - r#const: token_gas_cost(CostTokenType::Const) as u64, - pedersen: token_gas_cost(CostTokenType::Pedersen) as u64, - bitwise: token_gas_cost(CostTokenType::Bitwise) as u64, - ecop: token_gas_cost(CostTokenType::EcOp) as u64, - poseidon: token_gas_cost(CostTokenType::Poseidon) as u64, - add_mod: token_gas_cost(CostTokenType::AddMod) as u64, - mul_mod: token_gas_cost(CostTokenType::MulMod) as u64, - } - } -} - #[cfg(feature = "with-mem-tracing")] #[allow(unused_imports)] pub(crate) use self::mem_tracing::{ diff --git a/src/values.rs b/src/values.rs index 796933dec..213360111 100644 --- a/src/values.rs +++ b/src/values.rs @@ -1,6 +1,6 @@ -//! # Params and return values de/serialization +//! # JIT params and return values de/serialization //! -//! A Rusty interface to provide parameters to cairo-native entry point calls. +//! A Rusty interface to provide parameters to JIT calls. use crate::{ error::{CompilerError, Error}, diff --git a/tests/common.rs b/tests/common.rs index b35a85e18..649499276 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -391,6 +391,7 @@ pub fn run_vm_contract( .collect_vec() } +#[track_caller] pub fn compare_inputless_program(program_path: &str) { let program: (String, Program, SierraCasmRunner) = load_cairo_path(program_path); let program = &program; @@ -437,6 +438,7 @@ pub fn run_native_starknet_contract( /// the results automatically, triggering a proptest assert if there is a mismatch. /// /// Left of report of the assert is the cairo vm result, right side is cairo native +#[track_caller] pub fn compare_outputs( program: &Program, entry_point: &FunctionId, @@ -742,12 +744,8 @@ pub fn compare_outputs( .unwrap_or(false) }); assert_eq!( - vm_result - .gas_counter - .unwrap_or_else(|| Felt::from(0)) - .to_bigint(), - Felt::from(native_result.remaining_gas.unwrap_or(0)).to_bigint(), - "gas mismatch" + vm_result.gas_counter.unwrap_or_else(|| Felt::from(0)), + Felt::from(native_result.remaining_gas.unwrap_or(0)), ); let vm_result = match &vm_result.value { @@ -809,11 +807,7 @@ pub fn compare_outputs( }, }; - pretty_assertions_sorted::assert_eq!( - native_result.return_value, - vm_result, - "return value mismatch" - ); + pretty_assertions_sorted::assert_eq!(native_result.return_value, vm_result); Ok(()) }