How to use inversion in if() - might be a bug report ?

[vaclavpe]

Hello all,

I got portable WinAVR with avr-gcc version 4.3.2 and tried to compile simple code:

/////////////////////////////// code
#include <avr/io.h>
#define XTALFQ (8e+6)

volatile unsigned char var0x55, var0xaa;

void delay_ms(unsigned int ms)
{
  while(ms--) {
    unsigned short int i;
    for (i=100; i ; i-- ) {
      unsigned short int b;
      for(b=12*XTALFQ/12E6;b;b--) asm volatile("nop"::);
    }
  }
}

/************** main program ********************/
int main(void)
{
   var0x55 = 0x55;
   var0xaa = 0xaa;

  /* INITIALIZE */
   DDRC|= (1<<PC5);
   PORTC|= (1<<PC5);

  /* BLINK, BLINK ... */
  while (1) {
    /* invert PC5 */
    PORTC ^= (1<<PC5);

    // this code works
    if ( (var0xaa ^ 0xff) == var0x55) {
       delay_ms(100);
    } else {
       delay_ms(200);
    }
    // this code is buggy
    if ( (~var0xaa) == var0x55) {
       delay_ms(100);
    } else {
       delay_ms(200);
    }

  }
}
///////////////////////////////////////// end of code

The problem is with buggy code - when using '~', it gets expanded to 16 bits (however, in avr-gcc 3.4.6 it was the same problem as well) :
    // this code works
    if ( (var0xaa ^ 0xff) == var0x55) {
  84: 80 91 61 00 lds r24, 0x0061
  88: 90 91 60 00 lds r25, 0x0060
  8c: 80 95       com r24
  8e: 89 17       cp r24, r25
  90: f1 f4       brne .+60     ; 0xce <main+0x62>
  92: 09 c0       rjmp .+18     ; 0xa6 <main+0x3a>
...
    // this code is buggy
    if ( (~var0xaa) == var0x55) {
  e4: 80 91 61 00 lds r24, 0x0061
  e8: 20 91 60 00 lds r18, 0x0060
  ec: 90 e0       ldi r25, 0x00 ; 0
  ee: 80 95       com r24
  f0: 90 95       com r25
  f2: 30 e0       ldi r19, 0x00 ; 0
  f4: 82 17       cp r24, r18
  f6: 93 07       cpc r25, r19
  f8: f9 f4       brne .+62     ; 0x138 <main+0xcc>
  fa: 09 c0       rjmp .+18     ; 0x10e <main+0xa2>

Furthermore, the new .LST file looks really crazy and the resulting code is bigger ( 266 bytes from 4.3.2 compared to 130 bytes from 3.4.6).

Does anybody have a clue, what happened ?

Thank you in advance,
Vaclav


_______________________________________________
AVR-GCC-list mailing list
AVR-GCC-list@...
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list

[David Brown-4]

Vaclav Peroutka wrote:

> Hello all,
>
>     // this code works
>     if ( (var0xaa ^ 0xff) == var0x55) {
>        delay_ms(100);
>     } else {
>        delay_ms(200);
>     }
>     // this code is buggy
>     if ( (~var0xaa) == var0x55) {
>        delay_ms(100);
>     } else {
>        delay_ms(200);
>     }
>
>
> The problem is with buggy code - when using '~', it gets expanded to 16 bits (however, in avr-gcc 3.4.6 it was the same problem as well) :

The "bug" is in your code (alternatively, it is in the C standards!).
This is the way C works - before any arithmetic or logical operation,
and on various other occasions (such as in a if(), while() or switch()
test), any data unit smaller than an int gets promoted to an int.  That
means the "~" complement is done as a 16-bit int.  avrgcc is pretty good
(not perfect, but not bad) at removing unnecessary 16-bit code when it
doesn't affect the results of the operation.  In this case, however, the
16-bit promotion /does/ affect the working of the code, and it must be
kept there.

Use the "-Wextra" flag to get a warning about these things - you should
be using "-Wall -Wextra" at a minimum on all but legacy C code.

(Perhaps I should file an "enhancement request" for avr-gcc to issue an
error message if it is called without warning flags.)

>
> Furthermore, the new .LST file looks really crazy and the resulting code is bigger ( 266 bytes from 4.3.2 compared to 130 bytes from 3.4.6).
>
> Does anybody have a clue, what happened ?
>

Without knowing the compiler flags you used, it's difficult to tell.
However, in general later avr-gcc versions often generate larger code
because they do more aggressive inlining.  There are compiler flags that
counter this behaviour if you want to reduce code size - search in the
archives of this mailing list for examples.


mvh.,

David



_______________________________________________
AVR-GCC-list mailing list
AVR-GCC-list@...
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list

[David Brown-4]

Vaclav Peroutka wrote:
 > David,
 >

(Try to remember to post back to the list, rather than individual users,
unless it is specifically an off-list email.)

 >> The "bug" is in your code (alternatively, it is in the C standards!).
 >> This is the way C works - before any arithmetic or logical operation,
 >> and on various other occasions (such as in a if(), while() or switch()
 >> test), any data unit smaller than an int gets promoted to an int.  That
 > OK, I did not know that it is in C standard.
 >

Now you know!

For the future, it's polite to word your posts with the assumption that
/you/ have got it wrong, rather than implying the compiler (or library,
or other tools) are at fault.  The tools do have a few bugs, but user
error is more common.

 >> means the "~" complement is done as a 16-bit int.  avrgcc is pretty
 >> good (not perfect, but not bad) at removing unnecessary 16-bit code
 >> when it doesn't affect the results of the operation.  In this case,
 >> however, the 16-bit promotion /does/ affect the working of the code,
 >> and it must be kept there.
 >  From this point of view, if I have "if (++someUCharVariable) {}", the
 > result should be always true ? 0xff will be promoted to 0x00ff,
 > increased to 0x0100. But this will not happen in avr-gcc (at least in my
 > version). The ASM looks like this:
 >     if (++someUCharVariable) {
 >  e2:    80 91 60 00     lds    r24, 0x0060
 >  e6:    8f 5f               subi    r24, 0xFF    ; 255
 >  e8:    80 93 60 00     sts    0x0060, r24
 >  ec:        80 91 60 00     lds    r24, 0x0060
 >  f0:        88 23           and    r24, r24
 >  f2:        91 f2           breq    .-92         ; 0x98 <main+0x18>
 > which is correct from 8-bit view, but not from ANSI/ISO standard ? No
 > 16-bit expansion ?
 >

No, what happens is that the operation ++someUCharVariable is carried
out as 8-bit.  Then the result (0x00, after the overflow) is promoted to
16-bit and tested.  The compiler avoids generating the 16-bit
instructions here, since the results are the same.

 >> Without knowing the compiler flags you used, it's difficult to tell.
 >> However, in general later avr-gcc versions often generate larger code
 >> because they do more aggressive inlining.  There are compiler flags
 >> that counter this behaviour if you want to reduce code size - search
 >> in the archives of this mailing list for examples.
 > The new flag you probably have on mind is "-fno-inline-small-functions".

Other useful flags include "-fno-split-wide-types" and "--param
inline-call-cost=3" (as well as -Os, of course).  Results vary by
application - sometimes inlining small functions makes code
significantly smaller, sometimes significantly bigger.

Using --combine and -fwhole-program can often help too.

 > It does not however exist in older GCC 3.4.6. I would propose some
 > clever mechanism, which counts number of calls of small functions and
 > compares them with length of the function code. Then avr-gcc would make
 > a decision if it is better to inline the function or not. But I am not
 > compiler programmer. So I don't know if it is even possible.
 >

I think that is exactly what the "--param inline-call-cost=3" flag does.
  However, there are complications that make it difficult for the
compiler to get accurate function code lengths and inlining costs at an
early enough stage, so there will always be a need for some manual
tuning for space-critical code.  I expect, however, that at some point
the standard configuration for avr-gcc will be changed to have flags set
to give a bit smaller code by default.

 > Thank you for your explanation,
 > Vaclav
 >




_______________________________________________
AVR-GCC-list mailing list
AVR-GCC-list@...
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list