#77 patches to support gb/z80 bank switching

[Robert Ramey]

Attached please find the patches to the code I had to make in order to make bank switching work for the Gameboy/GameboyColor.

a) I didn't change the functionality as it is in the compiler. It works as described in the manual - to the extent it is described at all. That is, I did the minimum changes necessary to make it work.

b) I also removed the start/end extra global symbols which didn't add anything but clutter to the map listing in my opinion.

c) I've used the compiler - with the attached patches on a small test program for the Gameboy as well as a large "real world" program. This program has been run on the VGB simulator and downloaded into a cartridge which supports bank switching. I quite confident that it works as one would hope.

d) Personally, I think that bank switching support could be improved a lot with different bank syntax that would permit bank switching to be resolved at compile time. When I first built my "real" program it was very slow due to bank switching in interrupts (a facility I HAD to have). I was able to resolve it by modest reorganization of my code modules. But with a different system this would not have been necessary. As currently implemented - bank switch calls are made when code is in different modules even though the code is in the same bank.

e) Needs to be added to the documentation:

"#pragma bank <bank #>" is used to specify that all code and const data in the current module will be placed in the indicated memory bank."

f) The usage of bank switching requires support from the runtime library. Currently, this support is only available in the GameboyDevelopmentKit gbdk. Note that i have my own moderatly modified version of the gbdk which supports this. I would be willing to upload this somewhere - but I'm not sure where to do it.

Robert Ramey

[Philipp Klaus Krause]

Of course I can't speak for all developers.

Personally, I focus on issues in parts of SDCC that I know well. Those are the ones I can deal with efficiently, and where I'm unlikely to introduce new bugs (and if I happen to introduce one, I can easily fix it myself).

Bugs that I consider very important (e.g. silent generation of broken code), but don't feel like I'll be able to fix them myself anytime soon, I add to the list for the next release in the wiki, in the hope that it increases visibility to other developers.

[Maarten Brock]

The developers are not an official organization, but more an anarchy. Whoever wants to work on something just does so. There is a tendency to work on bugs first and feature requests later.

[Sergey Belyashov]

Applied after deep refactoring in [r11728].

[Tony Pavlov]

I'm sorry, you might made a mistake:

           emit2 ("call banked_call");
+          emit2 ("!db !bankimmeds", name);
           emit2 ("!dws", name);
-          emit2 ("!dw !bankimmeds", name);

isn't that makes generating:

call banked_call
.dw bSymbol
.dw Symbol

instead of

call banked_call
.dw Symbol
.dw bSymbol

which breaks backward-compatibility? what made you to change the order?

[Sergey Belyashov]

This order permits to write more optimized banked_call code. It is not mistake. Moreover,I prefer to use register passing of address and bank. So it is just proof of concept for now.

[Tony Pavlov]

you better write that optimized code first. :) for gbz80 it will be the same, except old codebase is broken:

banked_call::           ; Performs a long call.
    pop hl      ; Get the return address
    ldh a,(__current_bank)
    push    af      ; Push the current bank onto the stack
    ld  a,(hl+)     ; Fetch the call address
    ld  e, a
    ld  a,(hl+)
    ld  d, a
    ld  a,(hl+)     ; ...and page
    inc hl      ; Yes this should be here
    push    hl      ; Push the real return address
    ldh (__current_bank),a
    ld  (.MBC1_ROM_PAGE),a  ; Perform the switch
    ld  l,e
    ld  h,d
    rst 0x20        ; ___sdcc_call_hl
banked_ret::
    pop hl      ; Get the return address
    pop af      ; Pop the old bank
    ld  (.MBC1_ROM_PAGE),a
    ldh (__current_bank),a
    jp  (hl)

[Sergey Belyashov]

Fixed [r11729]

Also added compiler option --reg-bcall. But peephole eliminates register loading... So it is not work for now.

[Tony Pavlov]

why not that useful switch for gbz80 too?

[Sergey Belyashov]

Read change carefully. This switch for both Z80 and GBZ80.

[Tony Pavlov]

sorry, missed that. thank you.

[Tony Pavlov]

you might also use a:bc with "push bc/ret", that makes it compatible with __z88dk_fastcall instead of inventing your own conventions. a:bc is also sutable for gbz80 if you enable support of __z88dk_fastcall for that target.

[Sergey Belyashov]

push bc/ret has only one advantage - it uses always free registers. But switch program will be very pessimisic:

;we expect HL and DE should be passed as is to called proc
;we cannot use IY
___sdcc_bcall_abc::
  push af  ;reserve space for old_bank
  push af  ;reserve space for backed_ret
  push bc
  push hl
  ld   hl, #7
  add  hl, sp
  ldh  a, (__current_bank)
  ld   (hl-), a
  dec  hl
  ld   a, (hl)
  ld   (hl), #>banked_ret
  dec  hl
  ld   (hl), #<banked_ret
  ldh  (__current_bank), a
  pop  hl
  ret

same for Z80:

___sdcc_bcall_abc::
  push af  ;reserve space for old_bank
  push af  ;reserve space for backed_ret
  push bc
  push hl
  ld   hl, #7
  add  hl, sp
  in   a, (__current_bank)
  ld   (hl), a
  dec  hl
  dec  hl
  ld   a, (hl)
  ld   (hl), #>banked_ret
  dec  hl
  ld   (hl), #<banked_ret
  out  (__current_bank), a
  pop  hl
  ret

___sdcc_bcall_ciy::
   in   a, (__current_bank)
   push af
   ld   a, c
   out  (__current_bank), a
   call ___sdcc_call_iy
   pop  af
   out (__current_bank), a
   ret

[Tony Pavlov]

use a global variable for saving a and/or hl, that simplifies everything. you already have __current_bank, so that wont make things worse (i mean reentrancy and so on)

[Sergey Belyashov]

My project use banked calls for interrupt handler too. ;-)

[Tony Pavlov]

that will work only if you don't allow nested interrupts or don't use a threading library like this: https://github.com/untoxa/gameboy_preemptive_multitasking your code must be atomic between in and out. banked calls in interrupts - that's a bad idea anyway.

[Sergey Belyashov]

Project is very very old. And it very very huge. Many routines require to be called periodically with fixed interval So they are called from interrupt handler. These routines in separate bank each...
Main program uses banked call too. So it is possible that interrupt occurs in the banked_call.

[Tony Pavlov]

your project seems to disallow nested interrupts since that works. so there will be no problem.

think about other's people projects too. :) and about good/bad practice too. and compatibility as well.

as i said below, you may spoil hl or de or both within your private trampoline for your private project if you don't care about fastcall.

[Tony Pavlov]

for your own crt implementation you may also just ld e,a at the first line of your trampoline function. if you don't care about z88dk compatibility yourself.

[Philipp Klaus Krause]

A big feature like this (really any feature, but I guess it is more important for a big one) needs regression testing.

[Tony Pavlov]

we can write any tests you wish, but, please, fix that at last! :)

[Maarten Brock]

Why is the Z80 port using b<function-name> instead of (<function-name> >> 16) ? The linker is perfectly capable of generating 24 bit addresses when instructed to.

The mcs51 huge model uses this. You place a compiled source in a named segment (--codeseg / #pragma codeseg) and tell the linker where to place this segment (-Wl-b). See the 4.1.3 Bankswitching paragraph in the manual.

[Tony Pavlov]

please, don't! you break the compatibility with an old link-gbz80 linker, and that is essential for game boy, because the modern one is simply broken. fix the sdldgb linker (and makebin) first then!

[Maarten Brock]

Can you explain what is broken or point to a bug report about it?

[Tony Pavlov]

yes, i can. banked support is disabled in sdldgb by Philipp. there is also a hardcoded limit in makebin:

  if (gb && size != 32768)
    {
      fprintf (stderr, "error: only length of 32768 bytes supported for GameBoy binary.\n");
      return 1;
    }

that disallows you to workaround the problem somehow.

even if you write your own makebin, offsets inside _CODE_N are invalid, because all _CODE_N (also _DATA_N with similar problems) are concatenated instead of having 0x4000 (and 0xA000 for data) as base address for each.

and if you specify 0x4000 for every _CODE_N in commandline, then the code is simply overwritten.

so the only solution is to use an old link-gbz80 linker from y2k to get large working roms. this solution works perfectly and is a keystone for GBDK-2020 project: https://github.com/Zal0/gbdk-2020 which is very popular.

sdasgb objects are luckily (with some known issues, though) compatible with link-gbz80.

you have a desire to fix that? you are very welcome! :)

[Sergey Belyashov]

https://sourceforge.net/p/sdcc/bugs/2930/