optimize labels, jumps

crsn/src/asm/instr/flatten.rs

@@ -6,6 +6,7 @@ use crate::asm::data::literal::{Label, Value};
 use crate::asm::error::{AsmError, CrsnError};
 use crate::asm::instr::{Cond, Instr, Op, Routine};
 use crate::builtin::defs::BuiltinOp;
+use crate::builtin::defs::Barrier;
 
 /// A trait for something that can turn into multiple instructions
 pub trait Flatten {
@@ -57,7 +58,7 @@ impl Flatten for Vec<Box<dyn Flatten>> {
         for item in self.into_iter() {
             ops.extend(item.flatten(label_num)?);
         }
-        Ok(ops)
+        labels_to_skips(ops)
     }
 }
 
@@ -66,25 +67,32 @@ impl Flatten for Routine {
         let skip_label = Label::Numbered(label_num.fetch_add(1, Ordering::Relaxed));
 
         let mut ops : Vec<Op> = vec![
-            BuiltinOp::FarJump(skip_label.clone()).into(),
+            BuiltinOp::Barrier {
-            BuiltinOp::Barrier(Some(format!("Routine \"{}\" start", self.name).into())).into(),
+                kind: Barrier::Open(skip_label.clone()),
+                msg: Some(format!("proc {} start", self.name).into())
+            }.into(),
+
             BuiltinOp::Routine(self.name.clone()).into(),
         ];
 
         ops.extend(self.body.flatten(label_num)?);
 
-        ops.push(BuiltinOp::Barrier(Some(format!("Routine \"{}\" end", self.name).into())).into());
+        ops.push(
-        ops.push(BuiltinOp::FarLabel(skip_label).into());
+            BuiltinOp::Barrier {
+                kind: Barrier::Close(skip_label.clone()),
+                msg: Some(format!("proc {} end", self.name).into())
+            }.into()
+        );
 
-        numbered_labels_to_skips(ops)
+        labels_to_skips(ops)
     }
 }
 
 /// Convert jumps to relative skips
-fn numbered_labels_to_skips(ops: Vec<Op>) -> Result<Vec<Op>, CrsnError> {
+pub fn labels_to_skips(ops: Vec<Op>) -> Result<Vec<Op>, CrsnError> {
     let mut label_positions = HashMap::<Label, usize>::new();
     for (n, op) in ops.iter().enumerate() {
-        if let Op::BuiltIn(BuiltinOp::Label(name @ Label::Numbered(_))) = op {
+        if let Op::BuiltIn(BuiltinOp::Label(name)) = op {
             label_positions.insert(name.clone(), n - label_positions.len());
         }
     }
@@ -93,10 +101,10 @@ fn numbered_labels_to_skips(ops: Vec<Op>) -> Result<Vec<Op>, CrsnError> {
     let mut skipped = 0;
     for (n, op) in ops.into_iter().enumerate() {
         match op {
-            Op::BuiltIn(BuiltinOp::Label(Label::Numbered(_))) => {
+            Op::BuiltIn(BuiltinOp::Label(_)) => {
                 skipped += 1;
             }
-            Op::BuiltIn(BuiltinOp::Jump(target @ Label::Numbered(_))) => {
+            Op::BuiltIn(BuiltinOp::Jump(target)) => {
                 if let Some(dest) = label_positions.get(&target) {
                     let skip = *dest as isize - n as isize + skipped;
                     cleaned.push(Op::BuiltIn(BuiltinOp::Skip(Rd::new(RdData::Immediate(skip as Value)))));
@@ -104,7 +112,7 @@ fn numbered_labels_to_skips(ops: Vec<Op>) -> Result<Vec<Op>, CrsnError> {
                     return Err(CrsnError::Asm(AsmError::LabelNotDefined(target)));
                 }
             }
-            Op::BuiltIn(BuiltinOp::JumpIf(cond, target @ Label::Numbered(_))) => {
+            Op::BuiltIn(BuiltinOp::JumpIf(cond, target)) => {
                 if let Some(dest) = label_positions.get(&target) {
                     let skip = *dest as isize - n as isize + skipped;
                     cleaned.push(Op::BuiltIn(BuiltinOp::SkipIf(cond, Rd::new(RdData::Immediate(skip as Value)))));

crsn/src/asm/mod.rs

@@ -30,5 +30,5 @@ pub fn assemble(source: &str, parsers: Arc<Vec<Box<dyn CrsnExtension>>>) -> Resu
     }
     trace!("------------------------");
 
-    Ok(Program::new(ops, parsers))
+    Ok(Program::new(ops, parsers)?)
 }

crsn/src/builtin/defs.rs

@@ -2,9 +2,19 @@ use crate::asm::data::{Rd, RdObj, Wr};
 use crate::asm::data::literal::{DebugMsg, Label, RoutineName};
 use crate::asm::instr::{Cond, Op};
 
+#[derive(Debug)]
+pub enum Barrier {
+    /// Barrier that logically opens a section that cannot be jumped into, typically a routine
+    Open(Label),
+    /// Closing counterpart to the Open barrier
+    Close(Label),
+    /// Stand-alone barrier
+    Standalone
+}
+
 #[derive(Debug)]
 pub enum BuiltinOp {
-    /// Do nothing
+    /// Do nothing (costs one cycle)
     Nop,
     /// Stop execution
     Halt,
@@ -31,17 +41,20 @@ pub enum BuiltinOp {
     /// Exit the current routine with return values
     Ret(Vec<Rd>),
     /// Mark a routine entry point (call target).
-    /// Kept in the low level instruction file for position-independent code
+    /// The RoutineName struct includes its arity
     Routine(RoutineName),
-    /// Skip backward or forward
+    /// Skip backward or forward. The skip count can be defined by an argument.
     Skip(Rd),
     /// Skip if a flag is set
     SkipIf(Cond, Rd),
-    /// Deny jumps, skips and run across this address, producing a run-time fault with a message.
+    /// Deny jumps, skips and run across this address, producing a run-time fault.
-    Barrier(Option<DebugMsg>),
+    Barrier {
+        kind: Barrier,
+        msg: Option<DebugMsg>
+    },
     /// Generate a run-time fault with a debugger message
     Fault(Option<DebugMsg>),
-    /// Deallocate an object.
+    /// Deallocate an extension object.
     /// The object is released and the handle becomes invalid.
     Drop(RdObj),
     /// Copy value

crsn/src/builtin/exec.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 use crate::asm::data::{Rd, RdData};
 use crate::asm::data::literal::Addr;
 use crate::asm::instr::Cond;
-use crate::builtin::defs::BuiltinOp;
+use crate::builtin::defs::{BuiltinOp, Barrier};
 use crate::module::{EvalRes, OpTrait};
 use crate::runtime::fault::Fault;
 use crate::runtime::frame::StackFrame;
@@ -23,8 +23,24 @@ impl OpTrait for BuiltinOp {
                 /* this is nop, but without any cost - just markers */
                 res.cycles = 0;
             }
-            BuiltinOp::Barrier(msg) => {
+            BuiltinOp::Barrier {
-                return Err(Fault::Barrier {
+                kind: Barrier::Open(lbl),
+                ..
+            } => {
+                match program.find_far_label(lbl) {
+                    Ok(pos) => {
+                        res.cycles = 0;
+                        state.set_pc(pos);
+                    }
+                    Err(e) => {
+                        return Err(e);
+                    }
+                }
+            }
+            BuiltinOp::Barrier {
+                msg, ..
+            } => {
+                return Err(Fault::FaultInstr {
                     msg: msg.clone().unwrap_or_else(|| "BARRIER".into())
                 });
             }
@@ -44,26 +60,28 @@ impl OpTrait for BuiltinOp {
                 }
             }
             BuiltinOp::Jump(name) => {
-                match program.find_local_label(state.get_pc(), name) {
+                unimplemented!()
-                    Ok(pos) => {
+                // match program.find_local_label(state.get_pc(), name) {
-                        state.set_pc(pos);
+                //     Ok(pos) => {
-                    }
+                //         state.set_pc(pos);
-                    Err(e) => {
+                //     }
-                        return Err(e);
+                //     Err(e) => {
-                    }
+                //         return Err(e);
-                }
+                //     }
+                // }
             }
             BuiltinOp::JumpIf(cond, name) => {
-                if state.test_cond(*cond) {
+                unimplemented!()
-                    match program.find_local_label(state.get_pc(), name) {
+                // if state.test_cond(*cond) {
-                        Ok(pos) => {
+                //     match program.find_local_label(state.get_pc(), name) {
-                            state.set_pc(pos);
+                //         Ok(pos) => {
-                        }
+                //             state.set_pc(pos);
-                        Err(e) => {
+                //         }
-                            return Err(e);
+                //         Err(e) => {
-                        }
+                //             return Err(e);
-                    }
+                //         }
-                }
+                //     }
+                // }
             }
             BuiltinOp::FarJumpIf(cond, name) => {
                 if state.test_cond(*cond) {

crsn/src/builtin/parse.rs

@@ -8,7 +8,7 @@ use crate::asm::instr::Op;
 use crate::asm::parse::arg_parser::TokenParser;
 use crate::asm::parse::parse_data::{parse_constant_name, parse_label, parse_rd, parse_reg_alias, parse_value};
 use crate::asm::parse::sexp_expect::expect_string_atom;
-use crate::builtin::defs::BuiltinOp;
+use crate::builtin::defs::{BuiltinOp, Barrier};
 use crate::module::{CrsnExtension, ParseRes};
 
 #[derive(Debug, Clone)]
@@ -167,10 +167,13 @@ impl CrsnExtension for BuiltinOps {
             }
 
             "barrier" => {
-                BuiltinOp::Barrier(match args.next() {
+                BuiltinOp::Barrier {
-                    None => None,
+                    kind: Barrier::Standalone,
-                    Some(s) => Some(expect_string_atom(Some(s))?.into()),
+                    msg: match args.next() {
-                })
+                        None => None,
+                        Some(s) => Some(expect_string_atom(Some(s))?.into()),
+                    }
+                }
             }
 
             "fault" => {

crsn/src/runtime/program.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use crate::asm::data::literal::{Addr, Label, RoutineName};
 use crate::asm::instr::Op;
-use crate::builtin::defs::BuiltinOp;
+use crate::builtin::defs::{BuiltinOp, Barrier};
 use crate::module::CrsnExtension;
 use crate::runtime::fault::Fault;
 
@@ -12,25 +12,30 @@ pub struct Program {
     pub ops: Vec<Op>,
     pub extensions: Arc<Vec<Box<dyn CrsnExtension>>>,
     routines: HashMap<RoutineName, Addr>,
+    local_labels: HashMap<Label, Addr>,
     far_labels: HashMap<Label, Addr>,
-    barriers: Vec<Addr>,
+    /// Barriers from-to (inclusive).
+    /// Standalone barriers have both addresses the same.
+    barriers: Vec<(Addr, Addr)>,
 }
 
 impl Program {
-    pub fn new(ops: Vec<Op>, extensions: Arc<Vec<Box<dyn CrsnExtension>>>) -> Arc<Self> {
+    pub fn new(ops: Vec<Op>, extensions: Arc<Vec<Box<dyn CrsnExtension>>>) -> anyhow::Result<Arc<Self>> {
         let mut p = Self {
             ops,
             extensions,
             routines: Default::default(),
+            local_labels: Default::default(),
             far_labels: Default::default(),
             barriers: Default::default(),
         };
-        p.scan();
+        p.scan()?;
-        Arc::new(p)
+        Ok(Arc::new(p))
     }
 
     /// Find all the named things
-    fn scan(&mut self) {
+    fn scan(&mut self) -> anyhow::Result<()> {
+        let mut barrier_starts : HashMap<&Label, Addr> = HashMap::new();
         for (pos, op) in self.ops.iter().enumerate() {
             match op {
                 Op::BuiltIn(BuiltinOp::FarLabel(name)) => {
@@ -39,12 +44,45 @@ impl Program {
                 Op::BuiltIn(BuiltinOp::Routine(name)) => {
                     self.routines.insert(name.clone(), pos.into());
                 }
-                Op::BuiltIn(BuiltinOp::Barrier(_)) => {
+                Op::BuiltIn(
-                    self.barriers.push(pos.into());
+                    BuiltinOp::Barrier {
+                        kind: Barrier::Open(lbl), ..
+                    }
+                ) => {
+                    barrier_starts.insert(lbl, pos.into());
+                }
+                Op::BuiltIn(
+                    BuiltinOp::Barrier {
+                        kind: Barrier::Close(lbl),
+                        msg,
+                    }
+                ) => {
+                    if let Some(start_pos) = barrier_starts.remove(lbl) {
+                        self.barriers.push((start_pos, pos.into()));
+                        self.far_labels.insert(lbl.clone(), pos.into());
+                    } else {
+                        anyhow::bail!("Block barrier \"{:?}\" closed without being open!", msg);
+                    }
+                }
+                Op::BuiltIn(
+                    BuiltinOp::Barrier {
+                        kind: Barrier::Standalone,
+                        ..
+                    }
+                ) => {
+                    self.barriers.push((pos.into(), pos.into()));
                 }
                 _ => {}
             }
         }
+
+        if !barrier_starts.is_empty() {
+            anyhow::bail!("Some block barriers open without being closed!");
+        }
+
+        debug!("Program scanned: {:?}", self);
+
+        Ok(())
     }
 
     /// Read a program instruction at address
@@ -62,14 +100,28 @@ impl Program {
             std::mem::swap(&mut from, &mut to);
         }
 
-        for b in &self.barriers {
+        for (b0, b1) in &self.barriers {
-            if *b >= from && *b <= to {
+            if b0 != b1 {
-                if let Op::BuiltIn(BuiltinOp::Barrier(msg)) = self.read(*b) {
+                // block barrier that only partially intersects the jump
-                    return Err(Fault::JumpThroughBarrier {
+                if (*b0 >= from && *b0 <= to) != (*b1 >= from && *b1 <= to) {
-                        msg: msg.clone().unwrap_or("No msg".into())
+                    if let Op::BuiltIn(BuiltinOp::Barrier { msg, .. }) = self.read(*b0) {
-                    });
+                        return Err(Fault::JumpThroughBarrier {
-                } else {
+                            msg: msg.clone().unwrap_or("BLOCK BARRIER".into())
-                    unreachable!();
+                        });
+                    } else {
+                        unreachable!();
+                    }
+                }
+            } else {
+                // point barrier
+                if *b0 >= from && *b0 <= to {
+                    if let Op::BuiltIn(BuiltinOp::Barrier { msg, .. }) = self.read(*b0) {
+                        return Err(Fault::JumpThroughBarrier {
+                            msg: msg.clone().unwrap_or("POINT BARRIER".into())
+                        });
+                    } else {
+                        unreachable!();
+                    }
                 }
             }
         }
@@ -87,37 +139,5 @@ impl Program {
         self.far_labels.get(name).copied()
             .ok_or_else(|| Fault::NoSuchLabel { label: name.clone() })
     }
-
-    /// Find local label by name (label within a boundary-delimited region).
-    /// If more than one such label exists, the outcome is undefined.
-    pub fn find_local_label(&self, pc: Addr, name: &Label) -> Result<Addr, Fault> {
-        // TODO more efficient impl with look-up
-
-        for at in (0..=pc.0).rev() {
-            match &self.ops[at as usize] {
-                Op::BuiltIn(BuiltinOp::Label(lbl)) if lbl == name => {
-                    return Ok(at.into());
-                }
-                Op::BuiltIn(BuiltinOp::Barrier(_)) => {
-                    break;
-                }
-                _ => {}
-            }
-        }
-
-        for at in pc.0..(self.ops.len() as u64) {
-            match &self.ops[at as usize] {
-                Op::BuiltIn(BuiltinOp::Label(lbl)) if lbl == name => {
-                    return Ok(at.into());
-                }
-                Op::BuiltIn(BuiltinOp::Barrier(_)) => {
-                    break;
-                }
-                _ => {}
-            }
-        }
-
-        Err(Fault::NoSuchLabel { label: name.clone() })
-    }
 }
 

crsn/src/runtime/run_thread/state.rs

@@ -42,6 +42,7 @@ impl RunState {
 
     /// Set program counter - address of the next instruction to run
     pub fn set_pc(&mut self, pc: Addr) {
+        trace!("PC := {}", pc);
         self.frame.pc = pc;
     }
 

examples/proc_skip.csn

@@ -1,11 +1,24 @@
 (
+    (:label)
     (ld r0 1)
 
     ; the procedure is surrounded by a jump and a label
     (proc foo
+        (:label)
         (nop)
+        (j :label)
+        (ret)
+    )
+
+    (proc bar
+        (:label)
+        (nop)
+        (j :label)
         (ret)
     )
 
     (ld r0 2)
+    (halt)
+
+    (j :label)
 )

launcher/src/main.rs

@@ -13,8 +13,10 @@ use crsn::runtime::run_thread::{RunThread, ThreadToken};
 use crsn_arith::ArithOps;
 use crsn_screen::ScreenOps;
 use crsn_stacks::StackOps;
+use std::time::Duration;
 
 mod read_file;
+mod serde_duration_millis;
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 struct LogConfig {
@@ -23,10 +25,13 @@ struct LogConfig {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(default)]
 struct Config {
     log: LogConfig,
     #[serde(skip)]
     program_file: String,
+    #[serde(with="serde_duration_millis")]
+    cycle_time: Duration,
 }
 
 impl Default for Config {
@@ -37,6 +42,7 @@ impl Default for Config {
                 modules: Default::default(),
             },
             program_file: "".to_string(),
+            cycle_time: Duration::default(),
         }
     }
 }
@@ -55,17 +61,43 @@ impl AppConfig for Config {
     /// Add args to later use in the `configure` method.
     fn add_args<'a: 'b, 'b>(clap: clap::App<'a, 'b>) -> clap::App<'a, 'b> {
         // Default impl
-        clap.arg(
+        clap
-            clap::Arg::with_name("input")
+            .arg(
-                .value_name("FILE")
+                clap::Arg::with_name("input")
-                .help("Program to run")
+                    .value_name("FILE")
-                .required_unless("default-config")
+                    .help("Program to run")
-                .takes_value(true),
+                    .required_unless("default-config")
-        )
+                    .takes_value(true),
+            )
+            .arg(
+                clap::Arg::with_name("cycle")
+                    .long("cycle")
+                    .short("C")
+                    .value_name("MILLIS")
+                    .help("Cycle time (ms)")
+                    .validator(|s| {
+                        let t = s.trim();
+                        if t.is_empty() {
+                            Err("cycle time requires an argument".into())
+                        } else {
+                            if t.chars()
+                                .find(|c| !c.is_ascii_digit())
+                                .is_some() {
+                                Err("cycle time requires an integer number".into())
+                            } else {
+                                Ok(())
+                            }
+                        }
+                    })
+                    .takes_value(true),
+            )
     }
 
     fn configure(mut self, clap: &ArgMatches) -> anyhow::Result<Self> {
         self.program_file = clap.value_of("input").unwrap().to_string();
+        if let Some(c) = clap.value_of("cycle") {
+            self.cycle_time = Duration::from_millis(c.parse().unwrap());
+        }
         Ok(self)
     }
 }
@@ -88,13 +120,9 @@ fn main() -> anyhow::Result<()> {
 
     info!("Start runtime");
 
-    let thread = RunThread::new(
+    let args = &[];
-        ThreadToken(0),
+    let mut thread = RunThread::new(ThreadToken(0), None, parsed.clone(), Addr(0), args);
-        None,
+    thread.set_speed(config.cycle_time);
-        parsed.clone(),
-        Addr(0), // TODO find "main"?
-        &[], // TODO from CLI?
-    );
 
     let a = thread.start();
     // ...

launcher/src/serde_duration_millis.rs

@@ -0,0 +1,17 @@
+use serde::{self, Deserialize, Deserializer, Serializer};
+use std::time::Duration;
+
+pub fn serialize<S>(value: &Duration, se: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+{
+    se.serialize_u64(value.as_secs() * 1000 + value.subsec_millis() as u64)
+}
+
+pub fn deserialize<'de, D>(de: D) -> Result<Duration, D::Error>
+    where
+        D: Deserializer<'de>,
+{
+    let s: u64 = u64::deserialize(de)?;
+    Ok(Duration::from_millis(s))
+}