sdcc-devel

Re: [sdcc-devel] [sdcc:code] New commit [r13530] by maartenbrock

[Philipp K. K. <pk...@sp...>]

Am 21.06.22 um 10:25 schrieb Small Device C Compiler SVN repository:
> device/include/mcs51/mcs51reg.h,

Looks like this change makes mcs51 regression tests fail, as apparently 
the SBIT macro wants three parameters, but only gets two here.

Re: [sdcc-devel] [sdcc:code] New commit [r14325] by maartenbrock

[Philipp K. K. <pk...@sp...>]

Am 27.08.23 um 14:00 schrieb Small Device C Compiler SVN repository:
> * support/regression/gte/960830-1.c: deleted, is only relevant for i386

AFAIK, the way we handled such cases so far is:
Leave the C file to document that we aready looked into it (so we don'T 
have to look at it again when we merge further GCC tests into our 
testsuite).
Instead disable the test via GTE_SKIP_TESTS in Makefile.in.

Philipp

Re: [sdcc-devel] [sdcc:code] New commit [r14325] by maartenbrock

[Maarten B. <sou...@ds...>]

Philipp Klaus Krause schreef op 2023-08-27 14:08:
> Am 27.08.23 um 14:00 schrieb Small Device C Compiler SVN repository:
>> * support/regression/gte/960830-1.c: deleted, is only relevant for 
>> i386
> 
> AFAIK, the way we handled such cases so far is:
> Leave the C file to document that we aready looked into it (so we
> don'T have to look at it again when we merge further GCC tests into
> our testsuite).
> Instead disable the test via GTE_SKIP_TESTS in Makefile.in.

I was unaware of this policy.

And I would not have looked at it if it hadn't generated a warning.

The file was not disabled in GTE_SKIP_TESTS.

But would it not be easier to make that documented list explicit?
Then there would be no need to keep copies in our repository, which will
also get handled every time the regression tests are run.

Maarten

Re: [sdcc-devel] [sdcc:code] New commit [r14325] by maartenbrock

[Felix S. <fe...@sa...>]

On Sun, Aug 27, 2023 at 03:21:18PM +0200, Maarten Brock wrote:
> I was unaware of this policy.

It's not a policy yet. Perhaps it should be, but not all tests from gcc
have been cleaned up yet.

> And I would not have looked at it if it hadn't generated a warning.
> 
> The file was not disabled in GTE_SKIP_TESTS.

I think Philipp suggested to disable it, instead of deleting it.
Deleting it will just remove it from the radar, and it will be added
back later (and/or cause confusion, repetition..).

The documentation is in cases/Makefile.in: items in *_SKIP_TESTS come
with a reason why they are excluded.

The mechanism should be made more explicit: move entries over to
cases/skip, and "include skip" in cases/Makefile.in.

cheers
felix

Re: [sdcc-devel] [sdcc:code] New commit [r14325] by maartenbrock

[Maarten B. <sou...@ds...>]

Felix Salfelder schreef op 2023-08-27 15:34:
> On Sun, Aug 27, 2023 at 03:21:18PM +0200, Maarten Brock wrote:
>> I was unaware of this policy.
> 
> It's not a policy yet. Perhaps it should be, but not all tests from gcc
> have been cleaned up yet.
> 
>> And I would not have looked at it if it hadn't generated a warning.
>> 
>> The file was not disabled in GTE_SKIP_TESTS.
> 
> I think Philipp suggested to disable it, instead of deleting it.
> Deleting it will just remove it from the radar, and it will be added
> back later (and/or cause confusion, repetition..).
> 
> The documentation is in cases/Makefile.in: items in *_SKIP_TESTS come
> with a reason why they are excluded.
> 
> The mechanism should be made more explicit: move entries over to
> cases/skip, and "include skip" in cases/Makefile.in.
> 
> cheers
> felix

I was more thinking of listing those skipped files with a reason in a
separate file and not put them in the repository at all and thus not
have them in any Makefile. That way they do not take up space and
bandwidth nor consume cycles during the daily tests.

Maarten

Re: [sdcc-devel] [sdcc:code] New commit [r14325] by maartenbrock

[Felix S. <fe...@sa...>]

Hi Maarten.

On Tue, Sep 05, 2023 at 12:12:01PM +0200, Maarten Brock wrote:
> I was more thinking of listing those skipped files with a reason in a
> separate file

Yes, this is what I suggested in my previous email. Split it into
sections.

> and not put them in the repository at all and thus not
> have them in any Makefile. That way they do not take up space and
> bandwidth nor consume cycles during the daily tests.

The difference between the gcc tests and our copy should be reduced in
order to reduce maintenance and documentation overhead. Generally,
forking is easy, unforking is hard.

Perhaps this is a matter of style. The method should offer a simple
answer to the non-expert question "what are they doing here"? A diff
against upstream (here: gcc) gives a partial but definite answer.

A text document (stored whereever) containing test documentation will be
outdated quickly. Or forgotten. This is why a Makefile is best. You
probably wouldn't know about it if it wasn't a Makefile..

cheers
felix

Re: [sdcc-devel] [sdcc:code] New commit [r14358] by maartenbrock

[Philipp K. K. <pk...@sp...>]

Am 25.09.23 um 11:43 schrieb Small Device C Compiler SVN repository:

> * support/regression/tests/bug-3563.c: fixed warnings

We no longer see warnings here, but with the #ifdef commented out, we 
now get failures for mcs51-small-stack-auto and mcs51-large-stack-auto.

Philipp

Re: [sdcc-devel] [sdcc:code] New commit [r14358] by maartenbrock

[Maarten B. <sou...@ds...>]

Philipp Klaus Krause schreef op 2023-09-26 21:06:
> Am 25.09.23 um 11:43 schrieb Small Device C Compiler SVN repository:
> 
>> * support/regression/tests/bug-3563.c: fixed warnings
> 
> We no longer see warnings here, but with the #ifdef commented out, we
> now get failures for mcs51-small-stack-auto and
> mcs51-large-stack-auto.
> 
> Philipp

Thanks,

I had overlooked that I had also enabled the test for investigation.
I've reverted the comment out in r14360.

Maarten

Re: [sdcc-devel] [sdcc:code] New commit [r14923] by maartenbrock

[Philipp K. K. <pk...@sp...>]

Am 17.07.24 um 01:57 schrieb Small Device C Compiler (SDCC) SVN repository:
> * device/lib/z80/__itoa.s,
> * device/lib/z80/__ltoa.s,
> * device/lib/z80/__mulsint2slong.s,
> * device/lib/z80/__strreverse.s,
> * device/lib/z80/__uitobcd.s,
> * device/lib/z80/abs.s,
> * device/lib/z80/atomic_flag_test_and_set.s,
> * device/lib/z80/crt0.s,
> * device/lib/z80/divmixed.s,
> * device/lib/z80/divsigned.s,
> * device/lib/z80/divunsigned.s,
> * device/lib/z80/memcpy.s,
> * device/lib/z80/memmove.s,
> * device/lib/z80/modmixed.s,
> * device/lib/z80/modsigned.s,
> * device/lib/z80/modunsigned.s,
> * device/lib/z80/mul.s,
> * device/lib/z80/mulchar.s,
> * device/lib/z80/setjmp.s,
> * device/lib/z80/strcpy.s,
> * device/lib/z80/strlen.s: added .module <name> & .optsdcc -mz80 sdcccall(1)
> […]
> * src/z80/main.c (_z80_genAssemblerStart): new, added to emit .optsdcc,
> includes calling convention
> * src/z80/mappings.i: added .optsdcc

This means that we now get an "Internal Version Error" when trying to
link object files compiled with --sdcccall 1 to those compiled with
--sdcccall 0 (or objects from asm source files without the explicit
.optsdcc).

IMO, this is problematic, since it breaks compatibility. In particular,
we previously supported such use-cases:

test.c:
void f(void) __sdcccall(0);

void main(void)
{
}

test1.c:
void f(void)
{
}

Compiling test.c with -sdcccall 1 (i.e. the default), while compiling
test1.c with --sdcccall 0.
AFAIK, the main use case if libraries partially written in assembler to
the __sdcccalll(0) convention. By just adding __sdcccall(0) to
declarations in headers, these libraries were used in programs that
otherwise use the new default calling convention.

Philipp

Re: [sdcc-devel] [sdcc:code] New commit [r14923] by maartenbrock

[Maarten B. <sou...@ds...>]

Philipp Klaus Krause schreef op 2024-07-17 08:26:
> Am 17.07.24 um 01:57 schrieb Small Device C Compiler (SDCC) SVN 
> repository:
>> * device/lib/z80/__itoa.s,
>> * device/lib/z80/__ltoa.s,
>> * device/lib/z80/__mulsint2slong.s,
>> * device/lib/z80/__strreverse.s,
>> * device/lib/z80/__uitobcd.s,
>> * device/lib/z80/abs.s,
>> * device/lib/z80/atomic_flag_test_and_set.s,
>> * device/lib/z80/crt0.s,
>> * device/lib/z80/divmixed.s,
>> * device/lib/z80/divsigned.s,
>> * device/lib/z80/divunsigned.s,
>> * device/lib/z80/memcpy.s,
>> * device/lib/z80/memmove.s,
>> * device/lib/z80/modmixed.s,
>> * device/lib/z80/modsigned.s,
>> * device/lib/z80/modunsigned.s,
>> * device/lib/z80/mul.s,
>> * device/lib/z80/mulchar.s,
>> * device/lib/z80/setjmp.s,
>> * device/lib/z80/strcpy.s,
>> * device/lib/z80/strlen.s: added .module <name> & .optsdcc -mz80 
>> sdcccall(1)
>> […]
>> * src/z80/main.c (_z80_genAssemblerStart): new, added to emit 
>> .optsdcc,
>> includes calling convention
>> * src/z80/mappings.i: added .optsdcc
> 
> This means that we now get an "Internal Version Error" when trying to
> link object files compiled with --sdcccall 1 to those compiled with
> --sdcccall 0

Correct.

> (or objects from asm source files without the explicit .optsdcc).

Not correct. When there is no .optsdcc line the check is not performed.

> IMO, this is problematic, since it breaks compatibility. In particular,
> we previously supported such use-cases:
> 
> test.c:
> void f(void) __sdcccall(0);
> 
> void main(void)
> {
> }
> 
> test1.c:
> void f(void)
> {
> }
> 
> Compiling test.c with -sdcccall 1 (i.e. the default), while compiling
> test1.c with --sdcccall 0.

That is just plain stupid. I see no reason why SDCC should support that.
But for those who really want this, there is already the option
--no-optsdcc-in-asm which suppresses the .optsdcc line.

> AFAIK, the main use case if libraries partially written in assembler to
> the __sdcccalll(0) convention. By just adding __sdcccall(0) to
> declarations in headers, these libraries were used in programs that
> otherwise use the new default calling convention.
> 
> Philipp

Maarten