CJ's Project Blog

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.