Activity for Maarten Brock
-
Maarten Brock posted a comment on ticket #462
I can look into this later this week.
-
This is already reason for concern: $ ./configure --disable-z80-port --disable-z180-port \ --disable-r2k-port --disable-r2ka-port --disable-r3ka-port \ --disable-sm83-port --disable-tlcs90-port \ --disable-ez80-z80-port --disable-z80n-port --disable-r800-port \ --disable-ds390-port --disable-ds400-port --disable-pic14-port \ --disable-pic16-port --disable-hc08-port --disable-s08-port \ --disable-stm8-port --disable-pdk13-port --disable-pdk14-port \ --disable-pdk15-port --disable-mos6502-port \ --disable-mos65c02-port...
-
Conversion float to uint64_t is wrong
-
* support/regression/tests/wchar.c.in: bugfix
-
* support/regression/tests/wchar.c.in: added _FORTIFY_SOURCE==3 exception
-
* support/regression/cases/HTMLgen.py: fixed bad escape no longer accepted by Python 3.12
-
* device/lib/z80/__ultobcd.s: removed comments about odd ABI,
-
* device/lib/ds390/atomic_flag_test_and_set.c: renamed
-
* device/lib/ds390/__sdcc_atomic_maybe_rollback.c: bugfix moved push psw,
-
* device/include/setjmp.h: explicit __SDCC_mcs51 condition
-
* support/regression/tests/wchar.c.in: bugfix
-
* support/regression/tests/wchar.c.in: added _FORTIFY_SOURCE==3 exception
-
gen/host/wchar/wchar_test_wcharstringnorestart.bin > results/host/wchar/wchar_test_wcharstringnorestart.out Aborted (core dumped) It turns out that _FORTIFY_SOURCE==2 is no longer the default for GCC on Ubuntu, but has been changed to _FORTIFY_SOURCE==3 when optimization is enabled. So the exception made in wchar.c.in is no longer triggered.
-
I also saw this problem with Ubuntu 22.04 LTS. There the natively installed libboost-graph-dev is v1.74. Then I tried on Ubuntu 24.04 LTS and this problem went away due to the newer libboost-graph-dev v1.83. But Ubuntu 24.04 LTS also comes with GCC 13.2.0 which fails on regression test wchar_test_wcharstringnorestart, This did not fail with GCC 11.4.0 on Ubuntu 22.04 LTS.
-
I also saw this problem with Ubuntu 22.04 LTS. There the natively installed libboost-graph-dev is v1.74. Then I tried on Ubuntu 24.04 LTS and this problem went away due to the newer libboost-graph-dev v1.83. But Ununtu 24.04 LTS also comes with GCC 13.2.0 which fails on regression test wchar_test_wcharstringnorestart, This did not fail with GCC 11.4.0 on Ubuntu 22.04 LTS. gen/host/wchar/wchar_test_wcharstringnorestart.bin > results/host/wchar/wchar_test_wcharstringnorestart.out Aborted (core dum...
-
I also saw this problem with Ubuntu 22.04 LTS. There the natively installed libboost-graph-dev is v1.74. Then I tried on Ubuntu 24.04 LTS and this problem went away due to the newer libboost-graph-dev v1.83. But Ununtu 24.04 LTS also comes with GCC 13.2.0 which fails on regression test wchar_test_wcharstringnorestart, This did not fail with GCC 11.4.0 on Ubuntu 22.04 LTS.
-
If you can compile and link this with Keil then you probably selected the correct memory model there. The same will have to be done for SDCC. The default small memory model will try to put all variables in data memory which is max 120 bytes and thus 18866 will never fit. But even with --model-large you need 836 bytes for spilling of intermediate variables in those 120 bytes. Going one step further using --stack-auto it still needs more than 256 bytes on stack. Even --xstack will not help you here....
-
* support/regression/cases/HTMLgen.py: fixed bad escape no longer accepted by Python 3.12
-
I also saw this problem with Ubuntu 22.04 LTS. There the natively installed libboost-graph-dev is v1.74. Then I tried on Ubuntu 24.04 LTS and this problem went away due to the newer libboost-graph-dev v1.83. But Ununtu 24.04 LTS also comes with GCC 13.2.0 which fails on regression test wchar_test_wcharnorestart, This did not fail with GCC 11.4.0 on Ubuntu 22.04 LTS.
-
It seems the same as [#3744] and the same way to avoid it, i.e. with that build I've given there it doesn't crash: C:\tmp\0\3>compile.bat C:\tmp\0\3>sdcc -v SDCC : z80/ez80_z80/z80n TD- 4.4.1 #14899 (MINGW64) published under GNU General Public License (GPL) C:\tmp\0\3>sdas8051 main.s main.s:1: Error: <q> Expected a ','. main.s:2: Error: <u> undefined symbol encountered during assembly removing C:\tmp\0\3>pause Press any key to continue . . . Also note: sdas not sdar
-
At link time all objects that have a .optsdcc line must have equal contents in that line. This means that objects without .optsdcc will not be checked. You can suppress the line with --no-optsdcc-in-asm. It only tells what command line option was used (or the default). Explicit calling conventions on specific functions are not checked at link time. It prevents linking objects compiled with one calling convention with objects compiled with a different calling convention. It saves you if you forget...
-
I've used it in my code by declaring void __ultobcd (unsigned long v, unsigned char bcd[5]); found there. Ok, I will remove my observation then. And thanks for pointing to these exceptions. Done in [r14927].
-
* device/lib/z80/__ultobcd.s: removed comments about odd ABI,
-
You're right. Removed those again. Further, I noticed I forgot to commit ds390/atomic_flag_test_and_set.c . See [r14926]
-
* device/lib/ds390/atomic_flag_test_and_set.c: renamed
-
The additional dec r0 is not needed if push psw is moved down. Along with the above implemented in [r14923].
-
* device/lib/ds390/__sdcc_atomic_maybe_rollback.c: bugfix moved push psw,
-
The dec r0 bug is still present. I will fix this. I will also rename __sdcc_atomic_exchange_rollback_impl to sdcc_atomic_exchange_rollback_start. I see absolutely no reason why this implementation must be visible in the C namespace. I will add sdcc_atomic_exchange_rollback_end. I will add sdcc_atomic_exchange_exit for returning in DPL. sdcc_atomic_exchange_exit: mov dpl, r3 ret I will modify _atomic_flag_test_and_set to mov r2, #0x01 ljmp ___sdcc_atomic_exchange_gptr_impl
-
* device/include/setjmp.h: explicit __SDCC_mcs51 condition
-
I have now reread C23 7.13 again and my conclusion is that data does and buf does not need to be volatile for them to keep their values. So setjmp() needs special attention in the compiler to make sure it doesn´t push/pop intermediate pointers in iTemps over its invocation.
-
Bad optimization of condition handling
-
Sub-optimal code generation is not a bug. File this as a feature request next time, please. Further, I cannot reproduce this with version 4.4.2 #14916
-
Here is the atomic3.patch with many comments marked with >>>. It is not complete.
-
Here is the atomic3.patch with many comments marked with >>>.
-
Removing dec a is fine with me. It's not really restarting there either because it hasn't done anything yet. But this gives room for more optimizations as the 2 nops can be moved out of the routine. Further, we do not need to execute an extra nop when r3 contains the return value. Instead we can return earlier, placing the nop after the ret. Or even better, don´t use a call at all saving precious stack space. And we can drop another call if we rename the gptr entry point to _atomic_flag_test_and_set...
-
Since it is sometimes difficult for the compiler to prove that an ISR does not use atomics and thus adds the still expensive ISR epilogue for the possible rollback, it might be worthwhile to prevent the compiler from generating atomic support. The same goes for the atomic support routines. The simplest is to not use atomics if the used C standard is below C11. But maybe other keywords or pragmas are wished for as well.
-
What is the dec a for then? Is it not for skipping start-of-routine+#0 ? If you specify the return to be in r3, you can load dpl in the caller directly from r3. This only applies to the non-comparing case.
-
First review: Comment atomic_flag_test_and_set.c - C run-time: C11 atomic flag is wrong for the rollback. Leaving out the default areas is dangerous though less so for libraries. here0, etc are not proper local labels. There is no check for the return address lsb to be >= __sdcc_atomic_exchange_rollback_impl jc outer_skip should be jnc I think. cjne @r0, #6, here1 should be cjne a, #5, here1 jc inner_skip should be jnc I think. beign is not a word. There seems no need for mov a,r3 at the end of each...
-
Well, yes and no. __pdata does lie somewhere in __xdata space. But a (single byte) pdata pointer does not hold the page. And a (3 bytes) generic pointer does not either. It is expected instead that the page selection is active with the high byte of s_PSEG in __XPAGE (see crtxclear.asm) for MOVX @Ri to work.
-
Maybe we should try to keep those restartable functions in a single area and at power-of-two offsets (8?). The return address check can then be against the lower and upper bound and a simple AND operation to restart. Also what to roll back can be easier if the same registers are used at the same offsets. For mcs51 things can be more difficult. The pointer can be a generic pointer which first requires finding out the memory space. This can be done in a wrapper that calls the appropriate restartable...
-
I didn't dig deeper into the why, but apparently when data and buf are volatile their addresses are not saved on the stack, but rematerialized before use. And if I understand @spth correctly, they both should be volatile for the test to be valid.
-
Incorrect initialization of variables
-
Incorrect initialization of variables
-
This is not wrong. SDCC only initializes the explicitly given values and the rest is set to zero in crtclear.asm. https://sourceforge.net/p/sdcc/code/HEAD/tree/trunk/sdcc/device/lib/mcs51/crtclear.asm
-
The simple work around is not to initialize such global or static variables as the standard requires them to be initialized to zero anyway. But sure it would be nice if SDCC could detect that the initializer is all zeroes. Inefficient code generation is not a bug though.
-
Ticket moved from /p/sdcc/bugs/3756/ Can't be converted: _category: other
-
This can be considered a duplicate of [#3714] https://sourceforge.net/p/sdcc/bugs/3714/#e483/4c8a The workaround: I was able to install 32-bit 4.4.0 #14620 (MINGW32) observe sdar failing to start, but then download PortableGit-2.44.0-32-bit.7z.exe and unpack from it only: libwinpthread-1.dll libgcc_s_dw2-1.dll zlib1.dll From the \mingw32\bin of it, then put these three in the sdcc bin directory where sdar.exe is, and then sdar showed help etc. GNU ar (sdbinutils derived from GNU Binutils) 2.38
-
Many of the targets supported by SDCC are poorly equipped for reentrancy. That is why by default SDCC does not support reentrancy for them. The mcs51 and 6502 are both in that category, z180 is not. The reason this simple function works for mcs51 is pure luck, because the mcs51 has more registers that get used for intermediate results and do get saved/restored around calls. A bigger recursively called function would fail the same way. As Janko mentioned you can force SDCC to make functions reentrant,...
-
Ticket moved from /p/sdcc/bugs/3751/ Can't be converted: _category: MOS6502
-
Create a separate rule for ix/iy and place the offset in %4.
-
For the mcs51 this is native. It has separate instructions to read from code memory (movc) or external memory (movx) and also separate chip selects for external ROM and external RAM. I don't believe the 6502 has any such provisions. So the only option left is bank switching which is much harder and less efficient. I think the short answer is no.
-
This one has no notUsed('bc') but if BC still used after this code it now fails.
-
Where are all the notUsed clauses? Without these your peephole rules are going to cause a lot of trouble!
-
https://sourceforge.net/p/sdcc/code/HEAD/tree/trunk/sdcc/device/lib/mos6502/crt0.s shows that all vectors point to the same point. So you will have to create your own.
-
If IY is used so little in the libs, we might as well choose to always build them with --reserve-regs-IY. We can also choose to place the text "--reserve-regs-IY" or something similar in the .optsdcc to enforce compatibility. This will prevent linking objects with mixed reserve-regs settings.
-
Convesion float to uint64_t is wrong
-
I believe the intent was to divide by 128. Your implementation is to first shift 1 bit left and then 8 bits right. ADD HL,HL performs the shift left and places the original MSbit in the carry flag. Since this is the sign bit you must sign extend this bit in the MSByte H. Instead you shift the second MSbit into the carry using RLCA and sign extend that one into H. Using RLC H would also shift the second MSbit into the carry.
-
I don't think this is correct. The RLCA throws away the "sign of HL" in the carry flag. You also don't need to load A with L.
-
As for why not HL or why not DE, I think the register allocator cannot change the allocated register(s) halfway through the lifetime. So first the mulint result is stored in the registers chosen for the intermediate and then it is spilt to the stack and later restored.
-
The daily regression test log can be found on the snapshot page behind the red or green diamond in the right column under RT. https://sdcc.sourceforge.net/snap.php
-
In the char case you could just as easily swap the use of DE and HL and load HL simply with _array1.
-
Based on Maarten's code sample above, I now have this proposal, which should work and be reasonably efficient: ; Align the start of these 5 functions to an 8B boundary. Possibly by ; putting the code at the end of the IVT. Ensure that no 256B boundary ; is crossed. atomic_exchange_rollback_impl: atomic_exchange_pdata_impl: 0: movx a, @r0 ; 1 B 1a: mov r3, a ; 1 B 2a: nop nop ; 2 B 3a: mov a, r2 ; 1 B 4a: movx @r0, a ; 1 B 5: mov a, r3 ; 1 B 6: ret ; 1 B ;===== ; 8 B atomic_exchange_xdata_impl: 0:...
-
For mcs51 the call stack is always in __idata. Only parameters and local variables can go to __pdata for xstack. Although atomic_exchange_gptr_impl and atomic_exchange_idata_impl need not be placed at 8 byte offsets, they do need to be close to atomic_exchange_pdata_impl and atomic_exchange_xdata_impl since the jump on bit instructions use relative addressing. I suggest to keep them in the same asm file. The same goes for the _compare_ variants. The compare against #40 assumes that this code is 256...
-
Based on Maarten's code sample above, I now have this proposal, which should work and be reasonably efficient: ; Align the start of these 5 functions to an 8B boundary. Possibly by ; putting the code at the end of the IVT. Ensure that no 256B boundary ; is crossed. atomic_exchange_rollback_impl: atomic_exchange_pdata_impl: 0: movx a, @r0 ; 1 B 1a: mov r3, a ; 1 B 2a: nop nop ; 2 B 3a: mov a, r2 ; 1 B 4a: movx @r0, a ; 1 B 5: mov a, r3 ; 1 B 6: ret ; 1 B ;===== ; 8 B atomic_exchange_xdata_impl: 0:...
-
Based on Maarten's code sample above, I now have this proposal, which should work and be reasonably efficient: ; Align the start of these 5 functions to an 8B boundary. Possibly by ; putting the code at the end of the IVT. Ensure that no 256B boundary ; is crossed. atomic_exchange_rollback_impl: atomic_exchange_pdata_impl: 0: movx a, @r0 ; 1 B 1a: mov r3, a ; 1 B 2a: nop nop ; 2 B 3a: mov a, r2 ; 1 B 4a: movx @r0, a ; 1 B 5: mov a, r3 ; 1 B 6: ret ; 1 B ;===== ; 8 B atomic_exchange_xdata_impl: 0:...
-
R0 should be R3 or higher IMHO. See the pdata and _compare_ variants. This will require 16 bytes aligned routines and maybe that would finally be the first opportunity for SDCC to actually emit the XCHD A,@Ri instruction. ; roll back PC push ar1 push acc mov r1, SP dec r1 dec r1 dec r1 ; mov a,#0xF0 ; anl a,@r1 ; mov @r1,a clr a xchd a, @r1 ; clear low nibble of return address pop acc pop ar1
-
Well, yes and no. __pdata does lie somewhere in __xdata space. But a (single byte) pdata pointer does not hold the page. And a (3 bytes) generic pointer does not either. It is expected instead that the page is selection is active with the high byte of s_PSEG in __XPAGE (see crtxclear.asm) for MOVX @Ri to work.
-
Maybe we should try to keep those restartable functions in a single area and at power-of-two offsets (8?). The return address check can then be against the lower and upper bound and a simple AND operation to restart. Also what to roll back can be easier if the same registers are used at the same offsets. For mcs51 things can be more difficult. The pointer can be a generic pointer which first requires finding out the memory space. This can be done in a wrapper that calls the appropriate restartable...
-
I also get this with -mz80, [r14877] unsigned char some_param; unsigned char some_var; unsigned char some_func(unsigned char value); void main(void) { unsigned char some_return_value = some_func(some_param); some_var = 0; if (some_return_value == 0x10) { some_var = 1; } } produces here: _main:: ;check.c:6: unsigned char some_return_value = some_func(some_param); ld a, (_some_param) call _some_func ;check.c:7: some_var = 0; xor a, a ld (_some_var+0), a ;check.c:8: if (some_return_value == 0x10) {...
-
Seems it was actually fixed in [r14650]. So this ticket can be closed. There still seems to be a problem with case 3 generating a 16-bit sequence that seems redundant to me, given that the two input variables and the function parameter are all 8-bit. But that's not a functional bug - just a possible optimization, assuming the compiler can predict this.
-
How to know the segment number of a label in banked Roms
-
Suboptimal code generation is not a bug.
-
Ticket moved from /p/sdcc/bugs/3736/ Can't be converted: _category: other
-
Did you mark that variable volatile as I suggested?
-
this code will not compile in sdcc so you can only insert global variables, but I need a local variable Then first make it into something that does compile and inspect the generated asm. You'll find that the local variable is stored with a name that has both the function name (which you did not show) and the variable name. Also note that uint8 is not a standard type, so I can only assume that you gave it the typedef that I would expect.
-
Z80 is totally different from MCS51 in this regard unless the OP is using --stack-auto which he certainly did not mention.
-
The documentation for the linker is in asmlnk.txt SDCC creates a .lk file as the linker script when linking.
-
Is it possible in the new version of the compiler to insert local variable names into assembly code? while(1) { uint8 x=5; __asm mov a,x __endasm; }
-
Yes, start by looking at the generated asm to find the name of the variable. But I strongly suggest to mark this variable as volatile.
-
Personally, I always found the isaddr flag confusing. And I have the idea that some or all of the above (dis)advantages also hold when we drop this flag and introduce SET_VALUE_AT_ADDRESS. I like to see that something is a pointer in the specifier by a chain in the type instead of just an etype with some obscure flag set.
-
va_arg (printf) fail completly on target Z80
-
Should the optimal solution not use this? LD A,(DE) INC DE CPI JR NP, break JR NZ, loop
-
Peephole optimizer rule 1 should have eliminated that: replace restart { ld %1, %2 } by { ; common peephole 1 removed dead load from %2 into %1. } if notVolatile(%1), notUsed(%1), notVolatile(%2), notSame(%1 '(hl+)' '(hl-)'), notSame(%2 '(hl+)' '(hl-)') I guess we get a false negative on notUsed here, possibly notUsed accuracy suffers due to the jp fromt tail-call optimization; when I find time I might look into it to see if that is really the issue and if it is easy to improve notUsed accuracy here....
-
Suboptimal code generation is not a bug. It can be a feature request though to fix it.
-
Ticket moved from /p/sdcc/bugs/3724/ Can't be converted: _category: redundancy elimination
-
This is further handled through [features:#896]
-
Sorry for the late reply. The stack is normally placed at the end of internal memory and takes all that is left. Your problem however is that those OSEG areas need Direct memory which lives from 0x00 to 0x7F. The good thing is that they all Overlap (Oseg) so you only need to find space for 11 bytes.
-
This looks like a missed peephole optimization. Can you generate it again with --fverbose-asm so we can see which peephole rules were (not) applied?
-
This looks like a missed peephole optimization. Can you generate it again with --fverbose-asm so we can see which peephole rules were / weren
-
Is this just a tip about an approach you found somewhere or do you actually see generated code that can be improved? If the latter, can you give an example?
-
I guess this depends on whether we find a proper solution quickly. If so, there might be an intermediate release. And otherwise there will only be the win32 variant for Windows. It would be interesting to know about different build times between win32 and win64.
-
Linking libraries breaks arguments
-
Some options need to be used the same for compiling and for linking. The memory model and --stack-auto are the most notable ones. And unless you compile your own libraries I recommend not to use --float-reent and --int-long-reent.
-
Back-port error messages from asrab 5.40
-
What did you do to tell the linker how much memory you have? SDCC has no intrinsic knowledge about every derivative.
-
That does not help because your uint_8 casted value gets upcasted to an int for the ~ operator. You need to cast the inverted value down for this to work.
-
This happens because ~ upcasts the value to an int.
-
todos (to DOS) is the utility from tofrodos (to & from DOS) with DOS being MS-DOS or some such. It is used here to create the Windows .cmd batch file. It basically replaces every LF by CR/LF. An alternative is unix2dos. But this whole line looks like a difficult way to create that file IMHO. Why echo, then cat? And why not insert an escaped \r\n in the appropriate place?
-
I get the feeling that the larger memory space is mostly required for data and not for executable code. That would make it sensible to keep banked funtions in the 24 bit addressable memory and thus the virtual function pointer passed on stack a 24 bit value as well. The linker on the other hand should be able to link in a 32 bit address space. And yes, this might break external tools that rely on sdcc emitting 24 bit .rel files. Maybe those tools can also be made to handle both versions.
-
static const char *const moe = "-- --- ."; Using the second const will place the variable moe in code memory and so will the literal strings be placed there. But the first const does not change the pointer type from a generic (3-byte) pointer to a (2-byte) code pointer. So moe is a generic pointer here. Adding __code does turn it into a code pointer as you found out. static const char *const g_calls[5] = {moe, moi, mos, moh, mo5}; This defines an array of generic pointers and should not complain...
1 >