From 12e37d4335d611db0d27bfb999dd9b7a5b6f161e Mon Sep 17 00:00:00 2001 From: Nat Lasseter Date: Wed, 21 Oct 2020 19:57:14 +0100 Subject: [PATCH 1/3] Introduce the 'true' and 'false' conditions --- README.md | 3 +++ crsn/src/asm/instr/cond.rs | 11 +++++++++++ crsn/src/runtime/frame/status.rs | 2 ++ examples/test_cond.csn | 24 ++++++++++++++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 examples/test_cond.csn diff --git a/README.md b/README.md index fbc4ec9..42604ae 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,8 @@ These keywords (among others) are used in conditional branches to specify flag t - `nem`, `nempty` … Not empty - `eof` … EOF - `neof` … Not EOF +- `true`, `always`, `else` … Always true +- `false`, `never` … Always false # Syntax @@ -207,6 +209,7 @@ Conditonal branches are written like this: then only one branch is taken - there is no fall-through. - The definition order is preserved, i.e. if the `inval` flag is to be checked, it should be done before checking e.g. `nz`, which is, incidentally, true by default, because most flags are cleared by instructions that affects flags. +- `else` can be used as a final choice of branch that will always be taken. ## Routines diff --git a/crsn/src/asm/instr/cond.rs b/crsn/src/asm/instr/cond.rs index c3b86d3..13ff6c9 100644 --- a/crsn/src/asm/instr/cond.rs +++ b/crsn/src/asm/instr/cond.rs @@ -106,6 +106,10 @@ pub enum Cond { Eof, /// Not empty NotEof, + // Always true, for eg. (else? ...) + True, + /// Always false + False, } pub fn parse_cond(text: &str, pos: &SourcePosition) -> Result { @@ -134,6 +138,8 @@ pub fn parse_cond(text: &str, pos: &SourcePosition) -> Result { "neof" => Cond::NotEof, "ov" => Cond::Overflow, "nov" => Cond::NotOverflow, + "true" | "always" | "else" => Cond::True, + "false" | "never" => Cond::False, _ => { return Err(CrsnError::Parse(format!("Unknown cond: {}", text).into(), pos.clone())); } @@ -174,6 +180,8 @@ impl Display for Cond { Cond::Valid => "valid", Cond::Eof => "eof", Cond::NotEof => "neof", + Cond::True => "true", + Cond::False => "false", }) } } @@ -214,6 +222,9 @@ impl Not for Cond { Cond::NotEof => Cond::Eof, Cond::Eof => Cond::NotEof, + + Cond::True => Cond::False, + Cond::False => Cond::True, } } } diff --git a/crsn/src/runtime/frame/status.rs b/crsn/src/runtime/frame/status.rs index dec3d5a..286c897 100644 --- a/crsn/src/runtime/frame/status.rs +++ b/crsn/src/runtime/frame/status.rs @@ -115,6 +115,8 @@ impl StatusFlags { Cond::NotEmpty => !self.empty, Cond::Eof => self.eof, Cond::NotEof => !self.eof, + Cond::True => true, + Cond::False => false, } } diff --git a/examples/test_cond.csn b/examples/test_cond.csn new file mode 100644 index 0000000..b4a5208 --- /dev/null +++ b/examples/test_cond.csn @@ -0,0 +1,24 @@ +( + (ld r8 0) + + (ld r0 1 + (true? (add r8 1)) + ) + + (sub r0 1 + (pos? (fault)) + (else? (add r8 1)) + ) + + (nop + (always? (add r8 1)) + ) + + (nop + (never? (fault)) + ) + + (cmp r8 3 + (ne? (fault)) + ) +) From 0006cd06ece8dee97e67e85dbf570dbe52dd2ec6 Mon Sep 17 00:00:00 2001 From: Nat Lasseter Date: Thu, 22 Oct 2020 20:05:33 +0100 Subject: [PATCH 2/3] Reduce true/false conditionals down to just 'else' and warn if else is used in non-final branch --- crsn/src/asm/instr/cond.rs | 7 +++---- crsn/src/asm/instr/flatten.rs | 4 ++++ examples/test_cond.csn | 11 ++--------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/crsn/src/asm/instr/cond.rs b/crsn/src/asm/instr/cond.rs index 13ff6c9..5b72068 100644 --- a/crsn/src/asm/instr/cond.rs +++ b/crsn/src/asm/instr/cond.rs @@ -138,8 +138,7 @@ pub fn parse_cond(text: &str, pos: &SourcePosition) -> Result { "neof" => Cond::NotEof, "ov" => Cond::Overflow, "nov" => Cond::NotOverflow, - "true" | "always" | "else" => Cond::True, - "false" | "never" => Cond::False, + "else" => Cond::True, _ => { return Err(CrsnError::Parse(format!("Unknown cond: {}", text).into(), pos.clone())); } @@ -180,8 +179,8 @@ impl Display for Cond { Cond::Valid => "valid", Cond::Eof => "eof", Cond::NotEof => "neof", - Cond::True => "true", - Cond::False => "false", + Cond::True => "else", + Cond::False => "never", }) } } diff --git a/crsn/src/asm/instr/flatten.rs b/crsn/src/asm/instr/flatten.rs index b307b7c..d66721a 100644 --- a/crsn/src/asm/instr/flatten.rs +++ b/crsn/src/asm/instr/flatten.rs @@ -49,6 +49,10 @@ impl Flatten for InstrWithBranches { return Err(CrsnError::Asm(AsmError::ConditionalAlreadyUsed(cond), branch.pos())); } + if cnt != branch_count - 1 && cond == Cond::True { + warn!("\"Else\" conditional used in non-final branch at {}:{}", branch.pos().line, branch.pos().column); + } + let next_lbl = if cnt == branch_count - 1 { end_lbl.clone() } else { diff --git a/examples/test_cond.csn b/examples/test_cond.csn index b4a5208..fc45bce 100644 --- a/examples/test_cond.csn +++ b/examples/test_cond.csn @@ -2,7 +2,7 @@ (ld r8 0) (ld r0 1 - (true? (add r8 1)) + (else? (add r8 1)) ) (sub r0 1 @@ -10,15 +10,8 @@ (else? (add r8 1)) ) - (nop - (always? (add r8 1)) - ) - - (nop - (never? (fault)) - ) - (cmp r8 3 + (cmp r8 2 (ne? (fault)) ) ) From 8d10fa406d7e5ad42b004fe1c5550188b86afa12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Thu, 22 Oct 2020 21:55:00 +0200 Subject: [PATCH 3/3] Minor "else" improvements - Do not generate s.never to skip Else branch - Add Display impl to SourcePosition - Correct SourcePositions pointing to the end of a parsed expr instead of the beginning - Remove 'true', 'always', 'false' and 'never' from README.md --- README.md | 3 +-- crsn/crsn-sexp/src/error.rs | 6 ++++++ crsn/crsn-sexp/src/lib.rs | 5 +++-- crsn/src/asm/instr/flatten.rs | 14 ++++++++------ crsn/src/asm/instr/op.rs | 12 ++++++++---- examples/test_cond.csn | 19 +++++++++++++++++++ 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 42604ae..e5572db 100644 --- a/README.md +++ b/README.md @@ -98,8 +98,7 @@ These keywords (among others) are used in conditional branches to specify flag t - `nem`, `nempty` … Not empty - `eof` … EOF - `neof` … Not EOF -- `true`, `always`, `else` … Always true -- `false`, `never` … Always false +- `else` … Always true, may be used in the last branch # Syntax diff --git a/crsn/crsn-sexp/src/error.rs b/crsn/crsn-sexp/src/error.rs index e935717..cb5dea9 100644 --- a/crsn/crsn-sexp/src/error.rs +++ b/crsn/crsn-sexp/src/error.rs @@ -19,6 +19,12 @@ pub struct SourcePosition { pub index: u32, } +impl fmt::Display for SourcePosition { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "{}:{}", self.line, self.column) + } +} + /// Since errors are the uncommon case, they're boxed. This keeps the size of /// structs down, which helps performance in the common case. /// diff --git a/crsn/crsn-sexp/src/lib.rs b/crsn/crsn-sexp/src/lib.rs index 9eaf113..50815ab 100644 --- a/crsn/crsn-sexp/src/lib.rs +++ b/crsn/crsn-sexp/src/lib.rs @@ -331,10 +331,11 @@ fn parse_sexp(s: &str, pos: &mut usize) -> ERes { //trace!("parse_sexp {}", pos); zspace(s, pos)?; let (c, _) = peek(s, *pos)?; + let pos0 = *pos; let r = if c == '(' { - Ok(Sexp::List(parse_list(s, pos)?, spos(s, *pos))) + Ok(Sexp::List(parse_list(s, pos)?, spos(s, pos0))) } else { - Ok(Sexp::Atom(parse_atom(s, pos)?, spos(s, *pos))) + Ok(Sexp::Atom(parse_atom(s, pos)?, spos(s, pos0))) }; zspace(s, pos)?; r diff --git a/crsn/src/asm/instr/flatten.rs b/crsn/src/asm/instr/flatten.rs index d66721a..5af6a87 100644 --- a/crsn/src/asm/instr/flatten.rs +++ b/crsn/src/asm/instr/flatten.rs @@ -50,7 +50,7 @@ impl Flatten for InstrWithBranches { } if cnt != branch_count - 1 && cond == Cond::True { - warn!("\"Else\" conditional used in non-final branch at {}:{}", branch.pos().line, branch.pos().column); + warn!("\"Else\" conditional used in non-final branch at {}", branch.pos()); } let next_lbl = if cnt == branch_count - 1 { @@ -68,11 +68,13 @@ impl Flatten for InstrWithBranches { // optimization for single-branch conditionals with a single instruction ops.push(Op { cond: Some(cond), pos: pos.clone(), kind: flattened.remove(0).kind }); } else { - ops.push(Op { - kind: OpKind::BuiltIn(BuiltinOp::Jump(next_lbl.clone())), - pos: pos.clone(), - cond: Some(!cond), - }); + if cond != Cond::True { // evoid emiting `op.never` + ops.push(Op { + kind: OpKind::BuiltIn(BuiltinOp::Jump(next_lbl.clone())), + pos: pos.clone(), + cond: Some(!cond), + }); + } ops.extend(flattened); } diff --git a/crsn/src/asm/instr/op.rs b/crsn/src/asm/instr/op.rs index 61fdf92..b81799f 100644 --- a/crsn/src/asm/instr/op.rs +++ b/crsn/src/asm/instr/op.rs @@ -50,11 +50,15 @@ impl OpTrait for Op { OpKind::Ext(op) => op.to_sexp() }; + // TODO rewrite to be more readable? if let Some(cond) = self.cond { - if let Sexp::List(items, _) = &mut se { - if let Some(Sexp::Atom(Atom::S(s), _)) = &mut items.get_mut(0) { - s.push('.'); - s.push_str(&cond.to_string()); + // "true" is used for "else" branches, it has no effect - just omit it + if cond != Cond::True { + if let Sexp::List(items, _) = &mut se { + if let Some(Sexp::Atom(Atom::S(s), _)) = &mut items.get_mut(0) { + s.push('.'); + s.push_str(&cond.to_string()); + } } } } diff --git a/examples/test_cond.csn b/examples/test_cond.csn index fc45bce..30877d8 100644 --- a/examples/test_cond.csn +++ b/examples/test_cond.csn @@ -1,4 +1,23 @@ ( + ; test else + + (cmp 0 5 + (eq? (fault)) + (else? (ld r0 15)) + (lt? (fault)) ; This should produce a warning + ) + (cmp 15 r0 + (ne? (fault "else did not run"))) + + (ld r0 0) + + (cmp 0 5 + (lt? (nop)) + (else? (fault "fallthrough to else")) + ) + + ; + (ld r8 0) (ld r0 1