#373 Suite of extra tests for bitfields, stack and swap optimizations

[Visenri]

After many hours of testing I think I have generated an extensive and useful set of tests for these operations.

I attach the 7 different test files.
Many of them are quite massive in terms of cases.

Please check if all files are ok for you, If they are, I can upload my patches for the rules for STM8.
[patches:#362]
All the rules but 2 are used by the whole regression tests (and those can be commented or eliminated).
My tests cover 91% of the rules, the rest is covered by existing tests.

If we validate these tests, I hope it will be easier to validate the rules, even though I understand the rules quantity is quite massive.

[Visenri]

Some test cases require the use of absolute addresses, I know the addressing space of STM8 and what ranges are available, for the other platforms, these tests are run as what I call DUMMY tests (a test with no real code for testing).
This is handled like this:

#ifdef __SDCC_stm8
    #define ABS_ADDR1 0x5002
    #define ABS_ADDR2 0x5000
#endif

and later:

#ifdef ABS_ADDR2
   ------- definition of absolute address variables ---
#else
    #define DUMMY_CASE
#endif

Finally if DUMMY_CASE is defined, the whole test is processed as empty, just an empty test, no assertions.
Whoever is more familiar with the other platforms should fill one or two addresses for the other platfoms.

[Visenri]

One of the tests in stack-restore needs >256 bytes of stack, so it can't be used in all platforms.

I just added the same check I've seen used in other similar tests:

#if !defined(__SDCC_mcs51) && !defined(__SDCC_pdk14) && !defined(__SDCC_pdk15) && !defined(__SDCC_pic14) // Lack of memory

Excluding those platforms from the test, but I have not done any test beyond STM8 platform.

[Philipp Klaus Krause]

Thanks. I'll have a closer look later this week; I guess we'll adopt the tests with minor modifications (choosing suitable absolute addresses for some of the z80-related ports) then.

[Philipp Klaus Krause]

bitfields-bits, bitfields-checks, bitfields-nibbles have a lot of tests. Over 3000 for each of them. This seems excessive, especially considering that typically, there are some weaker machines in the compile farm and that SDC developers should run the tests before comitting. Would it be possible to reduce the number without compromising peephole rule coverage? bitfields-dead-loop also has a lot, but not as many as the other two.

For addresses that should work for other targets, see the test absolute.c (it is probably best to use addresses in RAM for tests, not in I/O space).

In [r12207], I added the three tests that do not use absolute addresses.

[Visenri]

I know, there are a lot of tests, I suffered it myself.
I just wanted to cover all possible cases.
This allowed me to pick a typo in one rule that was only a problem for some cases.

The rules themselves will be covered using far less tests, but not all combinations that lead to a similar final result will be covered.
For example, many rules apply several times for rotations and swaps until we end up in a "bset", "bres", "bcpl", but the combinations are different for each bit.
The same thing happens with nibbles tests, generating a single "or", "xor" or "and".

And these combinations are slightly different for each bit in the variable, and also sometimes the combinations change with the variable location (stack, global, local or absolute address), because the compiler output is not totally consistent.

I think that we will have to choose between speed or completeness if we reduce the number of cases.

Anyway, I'll have a look to see if I can improve the situation for bitfields-bits, bitfields-checks, bitfields-nibbles.

[Visenri]

For bitfields-bits I think there are some ways to reduce the number of tests without sacrificing completeness:

I can remove the cases of preBits = 16 (or even 8 bits), because I think it makes no difference to generated code, but I can't say for sure if this is true for all targets.
This will reduce the total tests by 1/3 for each removed preBits case.

I can also separate the functions:
bits_check_value_no_cast
bits_check_value

to another separate test with far less cases.

Then replace them in this file with much simpler versions using a cast to a type like:

typedef struct
{
    #if PRE_BITS > 0
        unsigned int preBits   : PRE_BITS;
    #endif
    unsigned char bits      : 8;
    unsigned char postBits   : 8;
} struct_8bits_joined

applied to the variable of the original struct type:

typedef struct
{
    #if PRE_BITS > 0
        unsigned int preBits   : PRE_BITS;
    #endif
    unsigned char bit0      : 1;
    unsigned char bit1      : 1;
    unsigned char bit2      : 1;
    unsigned char bit3      : 1;
    unsigned char bit4      : 1;
    unsigned char bit5      : 1;
    unsigned char bit6      : 1;
    unsigned char bit7      : 1;

    unsigned char postBits   : 8;
} struct_8bits;

This way the checks for the whole bits pack could be done in just one comparison:

((struct_8bits2_joined*)testVarPtr)->bits == value

instead of 8 of these:

if((var->bit0 > 0) != ((value & 0x01) > 0)) return 0;
...

This should speed it up by a factor of more than eight.

[Philipp Klaus Krause]

I also suggest to make this test portable by using different types for the bitfields: The C standard only mandates support for int, signed int, unsigned int, bool. While unsigned char currently kind of works in sdcc, I think there are some problematic corner cases (and we have no guarantee that the host compiler will support unsigned char for test-host, though AFAIK gcc and clang do).

[Visenri]

I applied several simplifications while trying to not sacrifice rule coverage, nor completeness:
Some obvious tests tested in any previous tests are not tested here (assigning values after casting struct of bitfields to byte).
bitfields-bits has been splitted in two as I said before.
Addresses have been updated to support other ports, following the addresses found in absolute.c .
Bitfilelds use unsigned int as suggested.

bitfields-bits1 has now 2016 tests, simulation time has been reduced to 1/3 of the original.
bitfields-bits2 : 72 tests.
bitfields-checks: 1344 tests.
bitfields-dead-loop: 14 tests.

"bitfields-nibbles" tests 5 different operations for 7 variable types using 2 patterns, so tests add up pretty quickly 70 x number of tests, I will see what I can do.

[Philipp Klaus Krause]

Thanks. With minor modifications, these 4 are now in [r12210].

[Visenri]

New bitfields-nibbles2.c with reduced number of cases.
(To execute the full test, uncomment the line with: #define FULL_TEST)

Disabled absolute addressing tests for:
-mcs51
-ds390

Disabled completely because of lack of memory (and also absolute absolute addressing issues):
-pdk

No problems expected with bitfields order (PPC & SPARC).

[Philipp Klaus Krause]

In [r12405] now also the last part has been put into trunk.