RE: [avr-gcc-list] 8 bit bitwise operation promoted to 16 bit values

[Neil Johnson]

Hi,

> > > Won't this always evaluate to false unless (LN_TX_BIT == LN_RX_BIT)?
>
> YES! That's why I did what I did.

Yes, I had a feeling this might be the case.  Since the the post askeed
nothing of correctnes I only tackled the question being asked :-)

> What it is testing for is for collisions on a bit-bashed serial network.
> So it sets the Tx pin and then a little later checks the Rx pin to see
> if it is following the state of the Tx pin. The Tx pin drives the base
> of an open collector transistor which inverts the signal, so if the pin
> states are the same there has been a collision. So it needs to know if
> the both pins are in the same state, be they 0 or 1 or not, hence the ==
> and not a &&.

Ok then, perhaps this might do the trick:

    if ( ( LN_TX_PORT ^ LN_RX_PORT ) & ( 0x01 << LN_TX_BIT ) )
    {
        ...
    }

Assuming that LN_TX_BIT == LN_RX_BIT, then the if block will only execute
if the two bits in LN_TX_PORT and LN_RX_PORT are the same.  This is a
shorter version of what David wrote, using the exclusive-OR operator ^.

I don't have a GCC AVR to hand, but I think this should compile to 8-bit
code.

> The problem in this case is the BitIndex in the for(;;) loop and Mask
> both use 16 bit operations.
> As you can see I have tried everything to tell it that Mask is only a
> byte
> ===============================
> static void updateFunctions5to8( THROTTLE_DATA_T *ThrottleRec, byte
> Func5to8, byte ForceNotify )
> {
>   byte Diffs ;
>   byte BitIndex ;
>   byte Mask ;
>
>   if( ForceNotify || ThrottleRec->Func5to8 != Func5to8 )
>   {
>     Diffs = ThrottleRec->Func5to8 ^ Func5to8 ;
>     ThrottleRec->Func5to8 = Func5to8 ;
>
>       // Check Functions 5-8
>     for( BitIndex = 0; BitIndex < 4; BitIndex++ )
>     {
>       Mask = (byte)((byte)1 << (byte)BitIndex) ;
>
>       if( ForceNotify || Diffs & Mask )
>         notifyThrottleFunction( ThrottleRec->UserData, (byte) (BitIndex
> + 5), Func5to8 & Mask ) ;
>     }
>   }
> }

Two suggestions spring to mind:

1) Do not recompute the mask inside the loop.  Instead, initialise Mask
outside the loop:
        byte Mask = 0x01;
then shift Mask inside the loop on each iteration:
        Mask <<= 1;

2) Persuade the optimizer that you want bytes by masking with 0xFF:

    if ( ( ForceNotify & 0xFF ) || ( Diffs & Mask & 0xFF ) )
    {
        ...
    }

The function argument ForceNotify would have been promoted to a 16-bit int
for passing to the function, so this might be getting in the way and
forcing the optimizer to keep the || as 16-bit.  Masking both sides with
0xFF should help the optimizer spot that the || can be done in 8-bits.

Give it a go.  As I said, at the moment I am unable to try these out
myself (that, and I have my own compiler to worry about!).

Cheers,
Neil

--
Neil Johnson :: Computer Laboratory :: University of Cambridge ::
http://www.njohnson.co.uk          http://www.cl.cam.ac.uk/~nej22
----  IEE Cambridge Branch: http://www.iee-cambridge.org.uk  ----