Issues when migrating back to Classic from newlib
Re: Issues when migrating back to Classic from newlib
It is possible you haven't done that at all, since I don't use SDCC, and no one was using SDCC+classic+sp1 anyway. So that never got tested, even if you have fixed it.dom wrote: ↑Tue Feb 22, 2022 6:47 pm Sometimes it becomes obvious that my attention span is shorter than it should be, see: viewtopic.php?p=19466#p19466
It looks like about a year ago I was updating the classic sp1 to use the newlib sp1 and solve the sdcc compatibility issue. However, it does look like I got a little distracted by splib2 instead and I can't find any trace of the sp1 work.
Not your fault of course. But I wouldn't have no idea how to do SDCC+classic+sp1, and I would have to install SDCC, which was a problem at that moment because of the downloading problems I've had with that special SDCC version.
Re: Issues when migrating back to Classic from newlib
So I'm the one Who gets to test It, a year later... Well, better late than never!
Re: Issues when migrating back to Classic from newlib
I had a very similar idea, but I'm not sure about where those asm files should go with the current Classic organization... I see in Newlib (under _DEVELOPMENT), the C stubs go into c/sccz80, c/sdcc_ix, etc. But there is no equivalent file organization in Classic, is there?
Shall the C stubs just go into the same directory as the implementation ones for now? That is, libsrc/sprites/software/sp1/spectrum/*...?
Re: Issues when migrating back to Classic from newlib
If code is used by both libs the source is just kept the once in newlib at the moment. You can see the pattern in ctype for example.
Re: Issues when migrating back to Classic from newlib
so they should be in... c/classic? or c/sdcc_classic? I'm not sure I get the naming and directory structure conventions here...
Re: Issues when migrating back to Classic from newlib
I've done the first stage in a branch since it's easier to show than explain: https://github.com/z88dk/z88dk/commit/1 ... f85490121b
Re: Issues when migrating back to Classic from newlib
OK, I see you just appended the new definitions to the existing ASM files (it was already visible in your script, but I skipped over it...)
Regarding the IX issue: since the newlib+SDCC combination is already covered in the current (master) source, wouldn't this mean that the IX issue has already been taken care of? The IX issue is with SDCC, but newlib SP1 already works with SDCC (it's my currently working config!) so there should be nothing else to do besides moving SP1 to the proper place, right?
I'll try a new Z88DK build and report back.
Regarding the IX issue: since the newlib+SDCC combination is already covered in the current (master) source, wouldn't this mean that the IX issue has already been taken care of? The IX issue is with SDCC, but newlib SP1 already works with SDCC (it's my currently working config!) so there should be nothing else to do besides moving SP1 to the proper place, right?
I'll try a new Z88DK build and report back.
Re: Issues when migrating back to Classic from newlib
@Dom, I added some modifications to your changes and managed to compile a new library "sp1-zx" (similar to the SP1 for other targets) for Newlib-SP1 on Classic, you can check the ongoing Pull Request (still in Draft). I've done it so that we can have both old-style-SP1 and Newlib-SP1 for Classic, compiled and active at the same time.
I have found a deeper issue (I think) regarding memory allocation by SP1: meanwhile in old-style-SP1 allocation was done via the u_malloc and u_free pointers (which are usually made to point to malloc and free, respectively), in Newlib-SP1 memory allocation is done by direct calls to asm_malloc.
The fix should be as simple as defining the proper DEFC for asm_malloc pointing to malloc (or asm_malloc_unlocked), but we can't directly do this: those definitions by default end up in asmHeapAlloc (for classic) and asm_heap_alloc (for newlib), and they have different interfaces: Classic's asmHeapAlloc expects HL=heap pointer and BC=allocation size; and Newlib's asm_heap_alloc expects HL=allocation size and DE=heap pointer.
I'll try to fix it by myself and add it to the PR, but any feedback on this would be welcome.
I have found a deeper issue (I think) regarding memory allocation by SP1: meanwhile in old-style-SP1 allocation was done via the u_malloc and u_free pointers (which are usually made to point to malloc and free, respectively), in Newlib-SP1 memory allocation is done by direct calls to asm_malloc.
The fix should be as simple as defining the proper DEFC for asm_malloc pointing to malloc (or asm_malloc_unlocked), but we can't directly do this: those definitions by default end up in asmHeapAlloc (for classic) and asm_heap_alloc (for newlib), and they have different interfaces: Classic's asmHeapAlloc expects HL=heap pointer and BC=allocation size; and Newlib's asm_heap_alloc expects HL=allocation size and DE=heap pointer.
I'll try to fix it by myself and add it to the PR, but any feedback on this would be welcome.
Re: Issues when migrating back to Classic from newlib
Sorry, slowing down a bit, the past week has been a bit frantic and I've got other non-z88dk things that need sorting out.
Looking at sp1_CreateSpr and the equivalent it looks like classic is expecting to a function with signature void u_malloc(int size) and the newlib version expects to call void asm_malloc(int size) __z88dk_fastcall;
However looking at the caller the parameter is pushed onto the stack so is compatible with both calling conventions. So I think we can just do this:
Looking at sp1_CreateSpr and the equivalent it looks like classic is expecting to a function with signature void u_malloc(int size) and the newlib version expects to call void asm_malloc(int size) __z88dk_fastcall;
However looking at the caller the parameter is pushed onto the stack so is compatible with both calling conventions. So I think we can just do this:
Code: Select all
IF __CLASSIC
call _u_malloc
ELSE
call asm_malloc
ENDIF
Re: Issues when migrating back to Classic from newlib
OK, so I did these mods and some related ones to Z88DK, now RAGE1 and its test game compiles and links successfully in 48K mode. Still pending: memory map issues (sections, pragmas, etc.), and make the test game work again (than means playable - not only compile ;-) ), some debugging is needed. And then, make sure 128K build also works.
I'll keep the PR still in draft and will add more changes to it in the next few days :-)
(Side note: I'm glad I switched to SDCC+Newlib+SP1 early when developing RAGE1 - If I had stood with Classic-SP1 and I had to fix all these things with the Z88DK knowledge I had a year ago, it would have been extremely frustrating. And with these SP1-porting days I'm learning loads of Z88DK... - Thanks @dom )
I'll keep the PR still in draft and will add more changes to it in the next few days :-)
(Side note: I'm glad I switched to SDCC+Newlib+SP1 early when developing RAGE1 - If I had stood with Classic-SP1 and I had to fix all these things with the Z88DK knowledge I had a year ago, it would have been extremely frustrating. And with these SP1-porting days I'm learning loads of Z88DK... - Thanks @dom )
Re: Issues when migrating back to Classic from newlib
Well, some news on this front: I have found a couple of (I think, serious) issues with SP1-NEWLIB sources compiled for classic-SDCC:
I'll report back.
- At first, I tried using the Classic SP1 headers (in arch/zx/sprites/sp1.h) with the NEWLIB sources. Everything linked OK, but the test programs crash on startup.
- Investigating this by looking at the generated ASM, I found that at least the sp1_Initialize function was expecting its params setup in a different way. I traced this to the point that the SDCC+NEWLIB SP1 includes defined the prototype for sp1_Initialize differently: in the Classic header, the params are uint8_t, and in the Newlib header thety are uint16_t
- I thought that this might be happening also to some other functions, so I cranked up a little script to compare SP1 function declarations for SDCC+NEWLIB, SCCZ80+NEWLIB and CLASSIC. Indeed this happens with other functions (e.g. sp1_CreateSpr, sp1_AddColSpr...)
I'll report back.
Re: Issues when migrating back to Classic from newlib
That makes sense - back when sdcc was added to classic I adjusted a lot of the headers to switch from a int8 to an int16 to work round a bug in sdcc at the time
However, there was a recentish fix to sdcc is meant to pad an int8 to int16 on the stack when the function is __smallc so it shouldn't be a problem anymore...
However, there was a recentish fix to sdcc is meant to pad an int8 to int16 on the stack when the function is __smallc so it shouldn't be a problem anymore...
Re: Issues when migrating back to Classic from newlib
Mmmm so according to this, what do you think should be the way forward now in regard to SP1 + Classic?dom wrote: ↑Sun Mar 13, 2022 9:30 pm That makes sense - back when sdcc was added to classic I adjusted a lot of the headers to switch from a int8 to an int16 to work round a bug in sdcc at the time
However, there was a recentish fix to sdcc is meant to pad an int8 to int16 on the stack when the function is __smallc so it shouldn't be a problem anymore...
- Just use the Classic-SP1 sources and patch them accordingly (if needed) so that they work with SDCC on Classic, then discard the Newlib-SP1 sources
- Backport the Newlib-SP1 sources (which work with SDCC and SCCZ80 on Newlib) and patch them so that they link and work with Classic, then discard the Classic-SP1 sources
Opinions?
Re: Issues when migrating back to Classic from newlib
I'm sorry, I've even distracted by real world things lately so I've not had much time for z88dk.
My preferred route is the backport - I assume that there may have been some bug fixes and it's also consistent with the way we do other things. I suspect either way there will some fiddly code changes needed.
My preferred route is the backport - I assume that there may have been some bug fixes and it's also consistent with the way we do other things. I suspect either way there will some fiddly code changes needed.
Re: Issues when migrating back to Classic from newlib
Right then, backport will be :-)
Re: Issues when migrating back to Classic from newlib
Hi, @dom, I'm stuck with some weird issue here. I've started porting the SP1 examples to the newlib-SP1 sources + SDCC as they would be a good testbed for the porting effort. New examples can be found (for the moment) under libsrc/sprites/software/sp1/zx/examples, and can be built for newlib-SP1 + Classic + SDCC by just doing "make" i that directory.
So far, ex1.c works OK with classic-SP1 + classic + SCCZ80 (of course :-), and newlib-SP1 + classic + SCCZ80 compiler. But it crashes when using newlib-SP1 + classic + SDCC compiler, and I really can't find the reason. I have checked the asm generated for the C source, the order of the parameters pushed on the stack, the parameters expected by the asm functions... everything seems in place, but I'm somehow getting some stack corruption.
To reproduce it, I changed ex1.c with the following lines (the final "while(1);" is there so tat I can inspect just the first couple of iterations):
And I get the following output:
It looks like the call to sp1_AddColSpr is corrupting the stack somehow ("i" is a local variable), and after that everything is wrong. Seems like a really simple matter of a calling convention somewhere, or something, but I can't find the problem...
Any ideas?
P.S. The files mentioned here can be found in the PR for the share_sp1 branch.
So far, ex1.c works OK with classic-SP1 + classic + SCCZ80 (of course :-), and newlib-SP1 + classic + SCCZ80 compiler. But it crashes when using newlib-SP1 + classic + SDCC compiler, and I really can't find the reason. I have checked the asm generated for the C source, the order of the parameters pushed on the stack, the parameters expected by the asm functions... everything seems in place, but I'm somehow getting some stack corruption.
To reproduce it, I changed ex1.c with the following lines (the final "while(1);" is there so tat I can inspect just the first couple of iterations):
Code: Select all
(...)
for (i=0; i!=10; i++) {
s = sprtbl[i].s = sp1_CreateSpr(SP1_DRAW_MASK2LB, SP1_TYPE_2BYTE, 3, 0, i);
+ printf( "i=%d r=%d c=%d w=%d h=%d\n", i, s->row, s->col, s->width, s->height );
sp1_AddColSpr(s, SP1_DRAW_MASK2, 0, 48, i);
+ printf( "i=%d r=%d c=%d w=%d h=%d\n", i, s->row, s->col, s->width, s->height );
sp1_AddColSpr(s, SP1_DRAW_MASK2RB, 0, 0, i);
+ printf( "i=%d r=%d c=%d w=%d h=%d\n", i, s->row, s->col, s->width, s->height );
sp1_MoveSprAbs(s, &cr, gr_window, 10, 14, 0, 4);
+ printf( "i=%d r=%d c=%d w=%d h=%d\n", i, s->row, s->col, s->width, s->height );
+ if ( i > 0 ) while (1);
};
(...)
It looks like the call to sp1_AddColSpr is corrupting the stack somehow ("i" is a local variable), and after that everything is wrong. Seems like a really simple matter of a calling convention somewhere, or something, but I can't find the problem...
Any ideas?
P.S. The files mentioned here can be found in the PR for the share_sp1 branch.
Re: Issues when migrating back to Classic from newlib
I mentioned this a while back.
The sp1 glue code needs to preserve ix or the sdcc frame pointer is corrupted.
The sp1 glue code needs to preserve ix or the sdcc frame pointer is corrupted.
Re: Issues when migrating back to Classic from newlib
THANKS, this was the clue I needed! I fixed the 8 SP1 functions used by ex1.c, and now it works ok!
Now I guess all glue functions should be modified to save IX as you said, this will be a big work... but indeed it is worth the time.
Just a guess: some of the functions also destroy IY... should the glue code also preserve IY? Or does SDCC treat IY just an additional register (i.e. no franme pointer?)
Re: Issues when migrating back to Classic from newlib
BTW, I'm keeping my PR constantly updated, so you can review my changes there.
Re: Issues when migrating back to Classic from newlib
IY is usually treated as an additional register - it seems to have two uses: as a general purpose register and as an "extended frame pointer" for when the stack frame is large. Either way, it's not necessary to preserve it.
Re: Issues when migrating back to Classic from newlib
Hey @dom, just finished fixing the IX issues in C glue functions. All SP1 examples also work (I discovered some needed fixes thanks to them), except ex5c which show some strange behaviour that I'm still investigating (but I'm not sure it's SP1 related).
Regarding saving IX where needed, I took the liberty of using register BC' for it, and so far it's worked OK.
Feel free to take a look at the PR, I removed the "draft" status, and all comments are welcome.
Regarding saving IX where needed, I took the liberty of using register BC' for it, and so far it's worked OK.
Feel free to take a look at the PR, I removed the "draft" status, and all comments are welcome.
Re: Issues when migrating back to Classic from newlib
Any chance to look at the PR, dom?
Re: Issues when migrating back to Classic from newlib
I think I’ve managed to muck up the PR - there’s hundreds of irrelevant changes which should already be in the target branch which keep breaking my browser before I can get to the toggle to view file by file.
I’ll take a look tonight - I was happy with where it was going originally so it can’t have gone too wrong since??
I’ll take a look tonight - I was happy with where it was going originally so it can’t have gone too wrong since??
Re: Issues when migrating back to Classic from newlib
I have already thought about It and I'm partially to blame, I have merged máster into my branch several times (I assumed you would not update the share_sp1 branch :-/ ).
I intend to do a new ( fixed) branch and a new PR with my changes only, so you can leave It to me until I Fix it.
I was mainly interested in you feedback, and now I have It. :-)
I'll keep you posted
I intend to do a new ( fixed) branch and a new PR with my changes only, so you can leave It to me until I Fix it.
I was mainly interested in you feedback, and now I have It. :-)
I'll keep you posted