From bd6847e8fc6013af7a18480dd50d997ac778af80 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Sat, 30 Jul 2022 21:51:55 -0700 Subject: [PATCH] Allow `%stack` to work with labels There's no syntax to distinguish named stack items from labels, so this simply searches the former first. I.e. labels can be shadowed by stack items. --- evm/src/cpu/kernel/assembler.rs | 14 ++++- evm/src/cpu/kernel/ast.rs | 5 +- evm/src/cpu/kernel/parser.rs | 2 +- evm/src/cpu/kernel/stack_manipulation.rs | 74 +++++++++++++++--------- 4 files changed, 65 insertions(+), 30 deletions(-) diff --git a/evm/src/cpu/kernel/assembler.rs b/evm/src/cpu/kernel/assembler.rs index 6d2b0ff2..5582b8e5 100644 --- a/evm/src/cpu/kernel/assembler.rs +++ b/evm/src/cpu/kernel/assembler.rs @@ -17,7 +17,7 @@ use crate::cpu::kernel::{ /// The number of bytes to push when pushing an offset within the code (i.e. when assembling jumps). /// Ideally we would automatically use the minimal number of bytes required, but that would be /// nontrivial given the circular dependency between an offset and its size. -const BYTES_PER_OFFSET: u8 = 3; +pub(crate) const BYTES_PER_OFFSET: u8 = 3; #[derive(PartialEq, Eq, Debug)] pub struct Kernel { @@ -505,8 +505,13 @@ mod tests { #[test] fn stack_manipulation() { let pop = get_opcode("POP"); + let dup1 = get_opcode("DUP1"); let swap1 = get_opcode("SWAP1"); let swap2 = get_opcode("SWAP2"); + let push_label = get_push_opcode(BYTES_PER_OFFSET); + + let kernel = parse_and_assemble(&["%stack (a) -> (a)"]); + assert_eq!(kernel.code, vec![]); let kernel = parse_and_assemble(&["%stack (a, b, c) -> (c, b, a)"]); assert_eq!(kernel.code, vec![swap2]); @@ -518,6 +523,13 @@ mod tests { consts.insert("LIFE".into(), 42.into()); parse_and_assemble_with_constants(&["%stack (a, b) -> (b, @LIFE)"], consts); // We won't check the code since there are two equally efficient implementations. + + let kernel = parse_and_assemble(&["start: %stack (a, b) -> (start)"]); + assert_eq!(kernel.code, vec![pop, pop, push_label, 0, 0, 0]); + + // The "start" label gets shadowed by the "start" named stack item. + let kernel = parse_and_assemble(&["start: %stack (start) -> (start, start)"]); + assert_eq!(kernel.code, vec![dup1]); } fn parse_and_assemble(files: &[&str]) -> Kernel { diff --git a/evm/src/cpu/kernel/ast.rs b/evm/src/cpu/kernel/ast.rs index 9580d9c6..b9d7286a 100644 --- a/evm/src/cpu/kernel/ast.rs +++ b/evm/src/cpu/kernel/ast.rs @@ -37,14 +37,15 @@ pub(crate) enum Item { #[derive(Clone, Debug)] pub(crate) enum StackReplacement { - NamedItem(String), + /// Can be either a named item or a label. + Identifier(String), Literal(Literal), MacroVar(String), Constant(String), } /// The target of a `PUSH` operation. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq, Hash)] pub(crate) enum PushTarget { Literal(Literal), Label(String), diff --git a/evm/src/cpu/kernel/parser.rs b/evm/src/cpu/kernel/parser.rs index f7acc96c..3c7b5fe2 100644 --- a/evm/src/cpu/kernel/parser.rs +++ b/evm/src/cpu/kernel/parser.rs @@ -112,7 +112,7 @@ fn parse_stack_replacement(target: Pair) -> StackReplacement { assert_eq!(target.as_rule(), Rule::stack_replacement); let inner = target.into_inner().next().unwrap(); match inner.as_rule() { - Rule::identifier => StackReplacement::NamedItem(inner.as_str().into()), + Rule::identifier => StackReplacement::Identifier(inner.as_str().into()), Rule::literal => StackReplacement::Literal(parse_literal(inner)), Rule::variable => { StackReplacement::MacroVar(inner.into_inner().next().unwrap().as_str().into()) diff --git a/evm/src/cpu/kernel/stack_manipulation.rs b/evm/src/cpu/kernel/stack_manipulation.rs index 63d0566c..a659fd35 100644 --- a/evm/src/cpu/kernel/stack_manipulation.rs +++ b/evm/src/cpu/kernel/stack_manipulation.rs @@ -5,7 +5,8 @@ use std::collections::{BinaryHeap, HashMap}; use itertools::Itertools; use crate::cpu::columns::NUM_CPU_COLUMNS; -use crate::cpu::kernel::ast::{Item, Literal, PushTarget, StackReplacement}; +use crate::cpu::kernel::assembler::BYTES_PER_OFFSET; +use crate::cpu::kernel::ast::{Item, PushTarget, StackReplacement}; use crate::cpu::kernel::stack_manipulation::StackOp::Pop; use crate::memory; @@ -22,22 +23,24 @@ pub(crate) fn expand_stack_manipulation(body: Vec) -> Vec { } fn expand(names: Vec, replacements: Vec) -> Vec { - let mut src = names.into_iter().map(StackItem::NamedItem).collect_vec(); - - let unique_literals = replacements + let mut src = names .iter() - .filter_map(|item| match item { - StackReplacement::Literal(n) => Some(n.clone()), - _ => None, - }) - .unique() + .cloned() + .map(StackItem::NamedItem) .collect_vec(); let mut dst = replacements .into_iter() .map(|item| match item { - StackReplacement::NamedItem(name) => StackItem::NamedItem(name), - StackReplacement::Literal(n) => StackItem::Literal(n), + StackReplacement::Identifier(name) => { + // May be either a named item or a label. Named items have precedence. + if names.contains(&name) { + StackItem::NamedItem(name) + } else { + StackItem::PushTarget(PushTarget::Label(name)) + } + } + StackReplacement::Literal(n) => StackItem::PushTarget(PushTarget::Literal(n)), StackReplacement::MacroVar(_) | StackReplacement::Constant(_) => { panic!("Should have been expanded already: {:?}", item) } @@ -49,7 +52,16 @@ fn expand(names: Vec, replacements: Vec) -> Vec src.reverse(); dst.reverse(); - let path = shortest_path(src, dst, unique_literals); + let unique_push_targets = dst + .iter() + .filter_map(|item| match item { + StackItem::PushTarget(target) => Some(target.clone()), + _ => None, + }) + .unique() + .collect_vec(); + + let path = shortest_path(src, dst, unique_push_targets); path.into_iter().map(StackOp::into_item).collect() } @@ -58,7 +70,7 @@ fn expand(names: Vec, replacements: Vec) -> Vec fn shortest_path( src: Vec, dst: Vec, - unique_literals: Vec, + unique_push_targets: Vec, ) -> Vec { // Nodes to visit, starting with the lowest-cost node. let mut queue = BinaryHeap::new(); @@ -93,7 +105,7 @@ fn shortest_path( continue; } - for op in next_ops(&node.stack, &dst, &unique_literals) { + for op in next_ops(&node.stack, &dst, &unique_push_targets) { let neighbor = match op.apply_to(node.stack.clone()) { Some(n) => n, None => continue, @@ -151,19 +163,23 @@ impl Ord for Node { #[derive(Eq, PartialEq, Hash, Clone, Debug)] enum StackItem { NamedItem(String), - Literal(Literal), + PushTarget(PushTarget), } #[derive(Clone, Debug)] enum StackOp { - Push(Literal), + Push(PushTarget), Pop, Dup(u8), Swap(u8), } /// A set of candidate operations to consider for the next step in the path from `src` to `dst`. -fn next_ops(src: &[StackItem], dst: &[StackItem], unique_literals: &[Literal]) -> Vec { +fn next_ops( + src: &[StackItem], + dst: &[StackItem], + unique_push_targets: &[PushTarget], +) -> Vec { if let Some(top) = src.last() && !dst.contains(top) { // If the top of src doesn't appear in dst, don't bother with anything other than a POP. return vec![StackOp::Pop] @@ -172,12 +188,12 @@ fn next_ops(src: &[StackItem], dst: &[StackItem], unique_literals: &[Literal]) - let mut ops = vec![StackOp::Pop]; ops.extend( - unique_literals + unique_push_targets .iter() - // Only consider pushing this literal if we need more occurrences of it, otherwise swaps + // Only consider pushing this target if we need more occurrences of it, otherwise swaps // will be a better way to rearrange the existing occurrences as needed. - .filter(|lit| { - let item = StackItem::Literal((*lit).clone()); + .filter(|push_target| { + let item = StackItem::PushTarget((*push_target).clone()); let src_count = src.iter().filter(|x| **x == item).count(); let dst_count = dst.iter().filter(|x| **x == item).count(); src_count < dst_count @@ -209,8 +225,14 @@ fn next_ops(src: &[StackItem], dst: &[StackItem], unique_literals: &[Literal]) - impl StackOp { fn cost(&self) -> u32 { let (cpu_rows, memory_rows) = match self { - StackOp::Push(n) => { - let bytes = n.to_trimmed_be_bytes().len() as u32; + StackOp::Push(target) => { + let bytes = match target { + PushTarget::Literal(n) => n.to_trimmed_be_bytes().len() as u32, + PushTarget::Label(_) => BYTES_PER_OFFSET as u32, + PushTarget::MacroVar(_) | PushTarget::Constant(_) => { + panic!("Target should have been expanded already: {:?}", target) + } + }; // This is just a rough estimate; we can update it after implementing PUSH. (bytes, bytes) } @@ -232,8 +254,8 @@ impl StackOp { fn apply_to(&self, mut stack: Vec) -> Option> { let len = stack.len(); match self { - StackOp::Push(n) => { - stack.push(StackItem::Literal(n.clone())); + StackOp::Push(target) => { + stack.push(StackItem::PushTarget(target.clone())); } Pop => { stack.pop()?; @@ -253,7 +275,7 @@ impl StackOp { fn into_item(self) -> Item { match self { - StackOp::Push(n) => Item::Push(PushTarget::Literal(n)), + StackOp::Push(target) => Item::Push(target), Pop => Item::StandardOp("POP".into()), StackOp::Dup(n) => Item::StandardOp(format!("DUP{}", n)), StackOp::Swap(n) => Item::StandardOp(format!("SWAP{}", n)),