Re: [avr-gcc-list] [avr-libc]: Smarter parity implementation

[Paulo Marques]

Georg-Johann Lay wrote:
> Paulo Marques wrote:
>> [...]
>> IIRC you're required to clear r25 even if the return type is uint8_t,
>> i.e., the return type needs to be upgraded to "int".
>>
>> I think this happens because, since most expressions are upgraded to int
>> anyway, it is cheaper to do it in one place than in all call sites.
> 
> The current parity_even_bit is not a function, it's just a macro that
> expands as
> 
> #define parity_even_bit(val)                          \
> (__extension__({                                      \
>       unsigned char __t;                              \
>       __asm__ (                                       \
>               "mov __tmp_reg__,%0" "\n\t"             \
>               "swap %0" "\n\t"                        \
>               "eor %0,__tmp_reg__" "\n\t"             \
>               "mov __tmp_reg__,%0" "\n\t"             \
>               "lsr %0" "\n\t"                         \
>               "lsr %0" "\n\t"                         \
>               "eor %0,__tmp_reg__"                    \
>               : "=r" (__t)                            \
>               : "0" ((unsigned char)(val))            \
>               : "r0"                                  \
>       );                                              \
>       (((__t + 1) >> 1) & 1);                         \
>  }))
> 
> the promotion is triggered by the last line, but there is
> no need for a premature promotion; the last line could be
> 
>    ((unsigned char) (__t & 1))
> 
> or
>    __t;
> 
> depending on what the asm does.
> 
> Doing the AND outside asm might be smart because tests like
> 
>   if (parity_even_bit (x))
> 
> get parts or condition code like Z-flag from AND.
> 
> 
> Anyway, even if parity_even_bit was a function
>    unsigned char parity_even_bit (unsigned car);
> there was no need to promote the return value.
> 
> The current text of the AVR ABI states that 8-bit return
> values are always promoted to int.
> 
> However, avr-gcc does not behave that way since 4.3+ and
> in earlier version the promotion was just because of an
> optimization flaw; there is/was no code in GCC's avr
> backend that describes/described such a promotion.
> 
> There was a discussion between some avr maintainers some
> time ago that addressed that issue.  The outcome was to
> update the ABI instead of forcing avr-gcc to promote.
> Updating the ABI will allow for better code generation,
> in particular in avr-libc's assembler parts.
> The only case when such an ABI change would trigger problems
> is when assembler code calls a C-function returning an 8-bit
> value and expects the return value to be promoted.
> 
> As avr-gcc actually does not promote since 4.3+ and there
> is not a single bug report in that direction (at least non
> I am aware of), I think it's very much preferred to adapt
> the ABI to avr-gcc's behavior than the other way round.
> 
> BTW, avr-gcc 4.7 comes with optimized versions of some buitins,
> e.g. parity:
> 
> unsigned char my_parity (unsigned char a, unsigned char b)
> {
>     return __builtin_parity (a) + b;
> }
> 
> which translates to
> 
> my_parity:
>       rcall __parityqi2
>       add r24,r22
>       ret
> 
> Notice that the compiler knows the register footprint, i.e.
> it knows that b (passed in r22) will survive the call.
> __parityqi2 uses the algorithm from above so that it might
> be desirable to use __builtin_parity when optimizing for size.

I just had a vague recollection of this issue and since I saw that the
previous version zero'ed r25 and yours didn't I thought it would be
better to point it out.

You seem to be more aware of this issue than I was, and I really
appreciated your thorough explanation of the current state of the art.

Thanks for taking the time to explain all this in such detail!

-- 
Paulo Marques
Software Development Department - Grupo PIE, S.A.
Phone: +351 252 290600, Fax: +351 252 290601
Web: www.grupopie.com

"To know recursion, you must first know recursion."