In software, as in most things, you seldom get it right on the first try.
(Ok, yeah, it’s July 20, the anniversary of a guy named Neil getting something right on the first try. But for the rest of us…)
In my last post, I had basic file and directory reading and writing working for my Retrochallenge project. If I’d had any sense, I would’ve continued on from there, added file writes, and been mostly done right now. Instead, I decided to refactor.
First, a warning: this entry features more than the usual amount of 6502 assembly for this blog, including a lot of what I’ve decided are bad ways to implement ideas. If you don’t know the difference between indirect indexed addressing and indexed indirect addressing – well, I have to look it up every time, too.
My code worked, but I found it aesthetically unappealing. In particular, I had some function table dispatch stuff that was just ugly. The idea is that I have a routine like getc, which takes a logical device number for an argument. It looks up that device number in my device table to find a pointer to the “real” getc implementation for that physical device, along with a channel number that I’m using to have multiple files open at once.
For example, the command channel for my SD card is logical device 1. It has a channel number of 0 (which tells the Arduino on the other side of the port that we’re sending a command instead of writing to a file), and there are device specific routines like SD_GETC and SD_PUTC which know how to talk to the Arduino over the parallel port.
My first attempt at implementing this in 6502 assembly language was less than elegant. I’m not really a sophisticated assembly hacker, so sometimes I overlook features that would make my life easier. I also tend to shoehorn high-level-language constructs into places they don’t really belong.
Trying to avoid that bad habit, and outsmarting myself, I implemented my device table as a bunch of arrays indexed by the device number – one array for the channel numbers for each logical device, one for the getc pointers, one for the putc pointers, and so on. I tried to do everything in one function, and that led me to this implementation:
; getc routine. Expects device number in A. .proc getc pha ; save dev number for later clc rol ; multiply by 2 for getc offset tax lda GETC_TAB,x ; get low byte of getc address sta SCRATCH lda GETC_TAB+1,x ; get high byte of getc address sta SCRATCH+1 plx ; get fd number for channel offset lda CHANNEL_TAB,x jmp (SCRATCH) ; jump to value in SCRATCH .endproc
Well, it worked, but the array indexing wasn’t as elegant as I intended. That’s because GETC_TAB is an array of 16-bit pointers and CHANNEL_TAB is an array of 8-bit numbers. So I have to shift the device number to get one offset, and save it or shift it back to get the other. Don’t even ask why I decided to store the device number on the stack instead of just rotating it back to the original value, because I can’t tell you.
I thought about splitting GETC_TAB (and PUTC_TAB, INIT_TAB, etc.) into separate arrays for the low and high bytes to simplify the indexing, but the ugliness of what I was doing was getting to me. Instead, I broke down and rearranged the device table into something that looked more like an array of C-style device structs. I figured I could generate a pointer to the struct for a particular device and save that somewhere that I could use it for indirect references to the function pointers. I also split things into two subroutines, set_device and getc, so that I didn’t have to juggle all the parameters in registers at once.
I ended up with this monstrosity:
; DEVTAB is an array of these DEVENTRY structs .struct DEVENTRY CHANNEL .byte INIT .word GETC .word PUTC .word padding .byte .endstruct ; Logical device number in A .proc set_device clc rol ; shift device number 3 bits rol ; so it becomes an index into rol ; devtab ; DEVTAB + index is address of our ; device struct. Save it to DEVICE_PTR adc #<DEVTAB sta DEVICE_PTR lda #>DEVTAB adc #0 ; just to add the carry bit sta DEVICE_PTR+1 ; Use DEVICE_PTR + offset to find channel number ldy #DEVENTRY::CHANNEL lda (DEVICE_PTR),y sta DEVICE_CHANNEL rts .endproc ; getc routine. Read a character from current device .proc getc ; copy DEVICE_PTR->GETC to scratch ldy #DEVENTRY::GETC_LO lda (DEVICE_PTR),y sta SCRATCH ldy #DEVENTRY::GETC_HI lda (DEVICE_PTR),y sta SCRATCH+1 jmp (SCRATCH) ; jump to value in SCRATCH .endproc
This is what I meant when I was talking about high-level-languages before. It may be more C-like, but I’m willing to say that this is objectively worse than the previous version. Then I noticed a feature of 6502 assembly language that I’d overlooked before. My P:65 computer uses a 65C02 CPU, which adds a few instructions to the original 6502. It also adds new addressing modes to existing instructions, such as an Absolute Indexed Indirect version of the JMP opcode.
Talk about a mouthful! What “Absolute Indexed Indirect” means is that it takes an address, adds the X register to it, and looks in that location for the address to jump to. It’s perfect for doing these structure table jumps. Once I rearranged the math, I realized I didn’t even need an explicit pointer to the DEVENTRY – I just needed to calculate the offset from the start of the table, which is the logical device number multiplied by the size of DEVENTRY struct. My code simplified to this:
; Logical device number in A .proc set_device clc rol ; shift device number 3 bits rol ; so it becomes an index into rol ; devtab sta DEVICE_OFFSET tax ; This next bit is the same as saying ; DEVTAB + DEVICE_OFFSET + CHANNEL lda DEVTAB + DEVENTRY::CHANNEL, X sta DEVICE_CHANNEL rts .endproc ; getc routine. Read a character from current device .proc getc ldx DEVICE_OFFSET jmp (DEVTAB + DEVENTRY::GETC, X) .endproc
Wow. I wish I’d written it that way in the first place! It just goes to show how important the refactoring process can be, especially when you’re learning your way around a language. It’s okay for your first solution to be hideous and overcomplicated, but you shouldn’t be afraid to go back and improve things when you’ve figured out a better way – especially in a hobby project.
Now, okay, a timed challenge like Retrochallenge may not be the best time to get caught up in refactoring, but I do think this will pay off over the next couple days as I try to call some of these routines from CC65‘s libc. I’ll be crossing my fingers as I get back to work.