diff options
| -rw-r--r-- | crates/nasm/README.md | 63 | ||||
| -rw-r--r-- | lib/xixanta/src/assembler.rs | 37 | ||||
| -rw-r--r-- | lib/xixanta/src/node.rs | 9 | ||||
| -rw-r--r-- | lib/xixanta/src/object.rs | 12 | ||||
| -rw-r--r-- | lib/xixanta/src/parser.rs | 57 | ||||
| -rwxr-xr-x | scripts/test-e2e.sh | 7 | ||||
| -rw-r--r-- | tests/expected/global-labels.nes | bin | 0 -> 32 bytes | |||
| -rw-r--r-- | tests/expected/global-labels.txt | 0 | ||||
| -rw-r--r-- | tests/global-labels.s | 26 |
9 files changed, 190 insertions, 21 deletions
diff --git a/crates/nasm/README.md b/crates/nasm/README.md index f96e0bb..1cfea8f 100644 --- a/crates/nasm/README.md +++ b/crates/nasm/README.md @@ -98,6 +98,69 @@ missing in `ca65`. In this case, `nasm` has the `--prelude` flag, which will print some code that can fill the gaps when running `ca65` with some nasm-specific code. See below. +#### Defining global labels + +For optimization reasons, sometimes it's necessary to write a label that can be +accessed globally, regardless of the current scope. The +[jetpac.nes](https://git.mssola.com/nes/jetpac.nes/) game has a good example of +this need on its `enemies.s` file. In there, we define a function pointer that +is set to the current function handler for the enemies' algorithm. Then enemy +handling can go along like this: + +```asm + ;; previous code + + lda #.hibyte(@return_from_movement_handler - 1) + pha + lda #.lobyte(@return_from_movement_handler - 1) + pha + jmp (zp_movement_fn) + +@return_from_movement_handler: + ;; Rest of the code after enemy movement has been handled. +``` + +That is, we push onto the stack the address of `@return_from_movement_handler`, +and then `jmp` to the function handler. Then, the function handler can simply +call `rts` and everything will be fine. As explained in the source code, other +techniques like trampolines or the "rts trick" were not desirable on this +context. + +Moreover, note that a simple `jmp` was not possible from the context of enemy +handlers, as `@return_from_movement_handler` is inside of the scope of the +`Enemies::update` proc. For this game the performance penalty with this setup +was acceptable, but note that it's inside of a loop, and these extra cycles are +given for each enemy. + +But this is potentially bad if your game is more tight on the cycle count and +every cycle you can squeeze matters. For this reason, `nasm` has the possibility +to define labels globally. That is, the above example could be rewritten like +this: + +```asm + jmp (zp_movement_fn) + +#@return_from_movement_handler: + ;; Rest of the code after enemy movement has been handled. +``` + +Notice the leading '#' symbol. This tells `nasm` that +`@return_from_movement_handler` should be defined at the global scope, +regardless of the current one (hence, it's no longer hidden inside of the +`Enemies::update` scope). With this, then each handler can switch from an `rts` +to: + +```asm + jmp @return_from_movement_handler ; or hide it on an 'RTS_FROM_ENEMY_HANDLER' macro. +``` + +In total, for each iteration loop, the setup would save 10 cycles (2 cycles x +lda, 3 cycles x pha), and replacing each handler's `rts` with a `jmp` would save +3 cycles (6 cycles 'rts' - 3 cycles 'jmp'). That is, given that this game +allowed 4 enemies at once, then each call to `Enemies::update` could save 13 +cycles x 4 enemies = 52 cycles. Not a crazy amount, yes, but this optimization +is now so easy to pull that it's well worth it. + #### The `__fallthrough__` pseudo-instruction It's a [well-known diff --git a/lib/xixanta/src/assembler.rs b/lib/xixanta/src/assembler.rs index 91f1e14..3cf1e0b 100644 --- a/lib/xixanta/src/assembler.rs +++ b/lib/xixanta/src/assembler.rs @@ -523,7 +523,10 @@ impl<'a> Assembler<'a> { accessed: 1, }; - if let Err(err) = self.context.set_variable(&var_name, &var_value, false) { + if let Err(err) = self + .context + .set_variable(&var_name, &var_value, false, false) + { Err(Error { global: true, line: 0, @@ -557,6 +560,7 @@ impl<'a> Assembler<'a> { accessed: 0, }, false, + matches!(node.node_type, NodeType::Label(true)), ) { return Err(Error { message, @@ -585,7 +589,7 @@ impl<'a> Assembler<'a> { // context so it's known. The actual value cannot be computed // right now as we don't know the segment size where it belongs // yet. - NodeType::Label => { + NodeType::Label(_) => { // There's no good reason to declare a named label inside of // a macro. If that's the case, just error out. if (self.macros_seen > 0 || self.repeats_seen > 0) && !node.value.is_empty() { @@ -689,6 +693,7 @@ impl<'a> Assembler<'a> { accessed: 0, }, false, + false, ) { errors.push(Error { message: err, @@ -909,7 +914,12 @@ impl<'a> Assembler<'a> { }; if !node.value.is_empty() - && let Err(message) = self.context.set_variable(&node.value, &object, true) + && let Err(message) = self.context.set_variable( + &node.value, + &object, + true, + matches!(node.node_type, NodeType::Label(true)), + ) { return Err(Error { message, @@ -938,7 +948,7 @@ impl<'a> Assembler<'a> { // segment. Note that this is only the offset from the beginning // of the offset, the effective address will only be available // after calling `Context::get_variable` - NodeType::Label => { + NodeType::Label(_) => { if let Err(e) = self.apply_segment_offset_to_label(node, ObjectType::Address) { errors.push(e); } @@ -1150,11 +1160,16 @@ impl<'a> Assembler<'a> { // value. let mut dobj = d.obj.clone(); dobj.bundle = o; - self.context.set_variable(&d.id, &dobj, true) + self.context.set_variable( + &d.id, + &dobj, + true, + matches!(n.node_type, NodeType::Label(true)), + ) } // No attached node, we can proceed by setting the // variable as is. - None => self.context.set_variable(&d.id, &d.obj, true), + None => self.context.set_variable(&d.id, &d.obj, true, false), }; } } @@ -1714,7 +1729,7 @@ impl<'a> Assembler<'a> { // calls, just in case a macro is applied multiple times and we // need to get the latest value. let id = &margs.next().unwrap().value; - if let Err(message) = self.context.set_variable(id, &obj, true) { + if let Err(message) = self.context.set_variable(id, &obj, true, false) { return Err(Error { line: node.value.line, message, @@ -2618,6 +2633,7 @@ impl<'a> Assembler<'a> { accessed: 0, }, true, + false, ) { return Err(Error { @@ -3099,7 +3115,12 @@ impl<'a> Assembler<'a> { // again. If the variable could not be set, then it's not // that big of a deal at this stage. value.bundle = bundle.clone(); - let _ = self.context.set_variable(&node.value, &value, true); + let _ = self.context.set_variable( + &node.value, + &value, + true, + matches!(node.node_type, NodeType::Label(true)), + ); // And return the computed bundle. Ok(bundle) diff --git a/lib/xixanta/src/node.rs b/lib/xixanta/src/node.rs index 649f333..357b9e0 100644 --- a/lib/xixanta/src/node.rs +++ b/lib/xixanta/src/node.rs @@ -319,8 +319,10 @@ pub enum NodeType { /// '$'. The `left` node contains the inner expression. Literal, - /// A label statement, which only sets the `value`, the name of the label. - Label, + /// A label statement, which only sets the `value`, the name of the + /// label. If the inner boolean is true, then the label is considered to be + /// global, otherwise scopes apply. + Label(bool), /// A macro call. Note that a Value might also encode this, but when a Call /// has been detected, then there is no doubt on it. @@ -347,7 +349,8 @@ impl fmt::Display for NodeType { NodeType::Control(control_type) => write!(f, "control function ({control_type})"), NodeType::ControlBody => write!(f, "control function body"), NodeType::Literal => write!(f, "literal"), - NodeType::Label => write!(f, "label"), + NodeType::Label(true) => write!(f, "(global) label"), + NodeType::Label(false) => write!(f, "(scoped) label"), NodeType::Call => write!(f, "call"), NodeType::Fallthrough => write!(f, "fallthrough"), NodeType::Operation(op) => match op { diff --git a/lib/xixanta/src/object.rs b/lib/xixanta/src/object.rs index 6209afd..cc8908c 100644 --- a/lib/xixanta/src/object.rs +++ b/lib/xixanta/src/object.rs @@ -329,14 +329,22 @@ impl Context { /// Sets a value for an object identified by `id`. If `overwrite` is set to /// true, then this value will be set even if the id already existed, - /// otherwise it will return a ContextError + /// otherwise it will return a ContextError. If 'force_global' is set to + /// true, then the variable will be defined at the global context, not the + /// current one. pub fn set_variable( &mut self, id: &PString, object: &Object, overwrite: bool, + force_global: bool, ) -> Result<(), String> { - let scope_name = self.name().to_string(); + let scope_name = if force_global { + GLOBAL_CONTEXT + } else { + self.name() + } + .to_string(); let scope = self.map.get_mut(&scope_name).unwrap(); match scope.get_mut(&id.value) { diff --git a/lib/xixanta/src/parser.rs b/lib/xixanta/src/parser.rs index abcf1e8..a71c6f6 100644 --- a/lib/xixanta/src/parser.rs +++ b/lib/xixanta/src/parser.rs @@ -536,7 +536,7 @@ impl Parser { // identifier and finally fall through. self.offset = 0; let (mut id, mut nt) = self.parse_identifier(l, true)?; - if nt == NodeType::Label { + if matches!(nt, NodeType::Label(_)) { self.nodes.last_mut().unwrap().push(PNode { node_type: nt, value: id, @@ -558,7 +558,7 @@ impl Parser { // for it and fall through. self.offset = 0; (id, nt) = self.parse_identifier(l, true)?; - if nt == NodeType::Label { + if matches!(nt, NodeType::Label(_)) { return Err(self .parser_error("cannot have multiple labels at the same location") .into()); @@ -624,10 +624,11 @@ impl Parser { // statement. Returns a PString representing this identifier on success, // plus a hint on whether the identifier belongs to a label or not. fn parse_identifier(&mut self, line: &str, dot: bool) -> Result<(PString, NodeType), Error> { - let start = self.column; - let base_offset = self.offset; + let mut start = self.column; + let mut base_offset = self.offset; let mut nt = NodeType::Value; let mut first_seen = false; + let mut global_label = false; // For the general case we just need to iterate until a whitespace // character or an inline comment is found. Then our PString object is @@ -645,6 +646,25 @@ impl Parser { if c == '.' && (!dot || first_seen) { return Err(self.parser_error("cannot have a '.' in this context")); } + + // If this is the first character, then allow a leading '#' + // character to denote a global label. Note that the '#' symbol will + // be skipped in the resulting name, and the start/end positions + // from the node will also reflect the '#' symbol being skipped. + if c == '#' && !first_seen { + global_label = true; + + // We don't want the '#' symbol to be considered part of the + // name, as it's just a literal from the syntax, not the name + // itself. + base_offset += 1; + start += 1; + + // Nothing else to be done, just jump into the next character. + first_seen = true; + self.next(); + continue; + } first_seen = true; // Check for the end of the identifier. For this, it's easier to @@ -732,18 +752,26 @@ impl Parser { } } // Regular label (e.g. "label:"). - _ => nt = NodeType::Label, + _ => nt = NodeType::Label(global_label), } } } // If there are no characters left, check if it was a regular label. None => { if c == ':' { - nt = NodeType::Label; + nt = NodeType::Label(global_label); } } } + // Detect bogus names starting with '#' but not being an actual + // label. + if global_label && !matches!(nt, NodeType::Label(true)) { + return Err( + self.parser_error("identifier starts with a '#' but it's not a label") + ); + } + // The value of the identifier is whatever we have picked up // along the parsing. let value = String::from(line.get(base_offset..self.offset).unwrap_or("").trim()); @@ -755,7 +783,7 @@ impl Parser { // regular labels because we want to ignore to extra ':' // character in the end. let end = match nt { - NodeType::Label => { + NodeType::Label(_) => { self.next(); self.column - 1 } @@ -1729,7 +1757,7 @@ impl Parser { return Err(self.parser_error("invalid identifier")); } - if nt == NodeType::Label { + if matches!(nt, NodeType::Label(_)) { Err(self.parser_error("not expecting a label defined here")) } else { Ok(PNode { @@ -2219,6 +2247,19 @@ mod tests { assert_node(nodes.last().unwrap(), NodeType::Instruction, line, "dex") } + #[test] + fn bad_pound_label() { + let mut parser = Parser::default(); + let err = parser + .parse("#fakelabel = 1".as_bytes(), &SourceInfo::default()) + .unwrap_err(); + + assert_eq!( + err.first().unwrap().message, + "identifier starts with a '#' but it's not a label" + ); + } + // Literals #[test] diff --git a/scripts/test-e2e.sh b/scripts/test-e2e.sh index 0620f25..6e18644 100755 --- a/scripts/test-e2e.sh +++ b/scripts/test-e2e.sh @@ -140,6 +140,13 @@ exit_code=$((exit_code + $?)) diff tests/out/fixed-segments.nes tests/expected/fixed-segments.nes exit_code=$((exit_code + $?)) +echo "test: custom => global-labels.nes" +./target/debug/nasm -c empty -Werror --asan -o tests/out/global-labels.nes tests/global-labels.s 2>tests/out/global-labels.txt +diff tests/out/global-labels.txt tests/expected/global-labels.txt +exit_code=$((exit_code + $?)) +diff tests/out/global-labels.nes tests/expected/global-labels.nes +exit_code=$((exit_code + $?)) + ## # code.nes diff --git a/tests/expected/global-labels.nes b/tests/expected/global-labels.nes Binary files differnew file mode 100644 index 0000000..3b24857 --- /dev/null +++ b/tests/expected/global-labels.nes diff --git a/tests/expected/global-labels.txt b/tests/expected/global-labels.txt new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/expected/global-labels.txt diff --git a/tests/global-labels.s b/tests/global-labels.s new file mode 100644 index 0000000..93b4d6d --- /dev/null +++ b/tests/global-labels.s @@ -0,0 +1,26 @@ +.segment "HEADER" + .byte 'N', 'E', 'S', $1A + .byte $02, $01 + .byte $00 + .byte $00 + +.segment "CODE" +;; asan:stack full + +.scope Scope + .proc foo + lda #1 ; $8000 + jmp Scope::bar ; $8002 + #@known_address: + rts ; $8005 + .endproc + + .proc bar + jmp @label ; $8006 + nop ; $8009 + @label: + jmp @known_address ; $800A + .endproc +.endscope + +jsr Scope::foo ; $800D |