kervinck / gigatron-rom

System, apps and tooling for the Gigatron TTL microcomputer
BSD 2-Clause "Simplified" License
235 stars 80 forks source link

asm.py: make real listing and reduce need for C('comment') #127

Closed kervinck closed 4 years ago

kervinck commented 4 years ago

The dissassembly can show comments when they are injected with the C() function. But this is a bit ugly: it is harder to read in the .py source than regular comments, while regular annotations don't show up in the .asm file. This leads to two views on the code. That isn't helpful.

A much better solution is to produce an old-fashioned listing of the source code, along with the assembly result on the left. This is possible with the Python inspect module!

I experimented a bit and the result can look like this:

                                                  1489  # Instruction LDW: Load word from zero page (vAC=[D]+256*[D+1]), 20 cycles
                                                  1490  label('LDW')
LDW:          0321 1200  ld   ac,x                1491  ld(AC,X)                        #10,12
              0322 8001  adda $01                 1492  adda(1)                         #11
              0323 c21d  st   [$1d]               1493  st([vTmp])                      #12 Address of high byte
              0324 0500  ld   [x]                 1494  ld([X])                         #13
              0325 c218  st   [$18]               1495  st([vAC])                       #14
              0326 111d  ld   [$1d],x             1496  ld([vTmp],X)                    #15
              0327 0500  ld   [x]                 1497  ld([X])                         #16
              0328 c219  st   [$19]               1498  st([vAC+1])                     #17
              0329 fc01  bra  NEXT                1499  bra('NEXT')                     #18
              032a 00f6  ld   $f6                 1500  ld(-20/2)                       #19
                                                  1501  

With this all C('') comments can be replaced with regular comments, except where inserted by a macro.

In a next phase, introspection opens the door for automatic verification of the cycle annotations.

kervinck commented 4 years ago

@psr You may want to benefit from this development. I hope I can wrap it up next weekend.

psr commented 4 years ago

I have definitely been worrying that the disassembly isn't going to be readable - and I've been taking the view that the Python code is the "real" source code, and the ASM doesn't really matter.

Your proposal will work well when most of the code is asm.py calls in a script as dev.py is, but if a lot of the code is generated through functions I think it might not add clarity (I'm really treating my code as a Python project rather than an ASM project).

For example, I'm trying to get to the point where I can write code like this:


def make_drop(name, size):
    @forthword(name=name, has_no_branches=True)
    def drop():
        label('forth.core.' + name)
        ld([variables.data_stack_pointer])
        adda(size)
        st([variables.data_stack_pointer])
    return drop

drop = make_drop('DROP', 2)
drop_two = make_drop('2DROP', 4)

and somewhere in dev.py I call drop() and drop_two() to emit the instructions

This is using helpers that I haven't written yet - but the idea is that forthword would do the instruction counting and insert the call to NEXT. In this case it's not clear what should go in the ASM - The Python responsible for a single instruction might be spread across three files - really the whole call stack is what you need!

I suppose what I'm saying is that for my purposes almost all of my code is going to be inserted by a macro.

kervinck commented 4 years ago

Ah yes, I see. I use the .asm files a lot to see what actually comes out. I think mostly to see at what addresses things land, and where the page boundaries are.

The notesTable comes from a function. It still has a C('')-comment for each generated item. In the prototype it gets listed like this:

                                                  2696  #-----------------------------------------------------------------------
                                                  2697  #
                                                  2698  #  $0900 ROM page 9: Key table for music
                                                  2699  #
                                                  2700  #-----------------------------------------------------------------------
                                                  2701  
                                                  2702  align(0x100, 0x100)
                                                  2703  notes = 'CCDDEFFGGAAB'
                                                  2704  sampleRate = cpuClock / 200.0 / 4
                                                  2705  label('notesTable')
notesTable:   0900 0000  ld   $00                 2706  ld(0)
              0901 0000  ld   $00                 2707  ld(0)
                                                  2708  for i in range(0, 250, 2):
                                                  2709    j = i/2-1
                                                  2710    freq = 440.0*2.0**((j-57)/12.0)
                                                  2711    if j>=0 and freq <= sampleRate/2.0:
                                                  2712      key = int(round(32768 * freq / sampleRate))
                                                  2713      octave, note = j/12, notes[j%12]
                                                  2714      sharp = '-' if notes[j%12-1] != note else '#'
                                                  2715      comment = '%s%s%s (%0.1f Hz)' % (note, sharp, octave, freq)
              0902 0045  ld   $45         ;C-0 (16.4 Hz)  2716      ld(key&127); C(comment)
              0903 0000  ld   $00                 2717      ld(key>>7)
              0904 0049  ld   $49         ;C#0 (17.3 Hz)
              0905 0000  ld   $00
              0906 004d  ld   $4d         ;D-0 (18.4 Hz)
              0907 0000  ld   $00
              0908 0052  ld   $52         ;D#0 (19.4 Hz)
              0909 0000  ld   $00
              090a 0056  ld   $56         ;E-0 (20.6 Hz)
              090b 0000  ld   $00
              090c 005c  ld   $5c         ;F-0 (21.8 Hz)
              090d 0000  ld   $00
              090e 0061  ld   $61         ;F#0 (23.1 Hz)
              090f 0000  ld   $00

The layout is still a bit messy on the transitions, that is the main reason I haven't committed it yet.

kervinck commented 4 years ago

Here a snippet with a wait() macro in it:

              0b2d fc32  bra  .sysSb#38           3063  bra('.sysSb#38')                #36
              0b2e de00  st   [y,x++]             3064  st([Y,Xpp])                     #37
                                                  3065  label('.sysSb#35')
.sysSb#35:    0b2f 0200  nop                      3066  wait(38-35)                     #35
              0b30 0200  nop
              0b31 0200  nop
                                                  3067  label('.sysSb#38')
.sysSb#38:    0b32 0124  ld   [$24]               3068  ld([sysArgs+0])                 #38
              0b33 2001  anda $01                 3069  anda(1)                         #39
psr commented 4 years ago

I like your wait snippet - it's good to see the expression.

I haven't fully worked out how to handle the page-boundaries yet - they are going to be important, as I need trampoline code in a fixed location on each ROM page. Ideally I'd solve the knapsack problem to find an optimal packing!

For now I'm trying to do one piece at a time, and commit each part separately. The diff then shows me what's changed, alongside the code that generated it. I also use the disassembly in PDB, which is really helpful - but usually my error is that I've written st(AC, X) instead of ld(AC, X)!

kervinck commented 4 years ago

The 2nd argument to the align() macro sets the size after which I want an error. This is how I guard unintentional page crossings in the kernel part.

psr commented 4 years ago

Aha! I had struggled with that a bit - it wasn't clear to me what the second argument was meant to mean - although from use in dev.py I could see that it needed to be 0x100 for my purposes.

kervinck commented 4 years ago

Good point. I'll rename the argument 'size' and use it explicitly where used. That makes it more self-documenting

kervinck commented 4 years ago

Solved with https://github.com/kervinck/gigatron-rom/commit/b2d63a325e59f047e0614ed5e0466a6ad54a6442

The commit comment suggests a speed improvement is possible "with some refactoring". But that is not that clear at all (probably false).