gcc hates me and how i know

[mtbf0]

i'm working on an boarduino brain machine. the code to generate the binaural beats is similar to this project. i spent two weeks trying to figure out a really peculiar bug and today after printing out some strange intermediate results the damnedest thing turned up.

given the following declarations for a bunch of 24 bit fixed point values...

Code: Select all

struct notestep{                // type for a 24 bit fixed point value
  uint8_t integer;
  uint16_t fraction;
};

const notestep basestep = { 3, 0x46dc };    // 200Hz
notestep offsetstep = { 3, 0x70cd };      // 210Hz
notestep baseaccumulator;
notestep offsetaccumulator;

this works...

Code: Select all

  offsetaccumulator.fraction += offsetstep.fraction;
  if (SREG & (1 << SREG_C))        // check for carry
    offsetaccumulator.integer++;
  offsetaccumulator.integer += offsetstep.integer;

and this doesn't ...

Code: Select all

  baseaccumulator.fraction += basestep.fraction;
  if (SREG & (1 << SREG_C))        // check for carry
    baseaccumulator.integer++;
  baseaccumulator.integer += basestep.integer;

i finally printed out enough intermediate values to figure out that the carries were not happening when they should, so i disassembled the binary and it turned out that with baseoffset declared as a constant the addition compiles to a subtraction, as if i had written ...

Code: Select all

  baseaccumulator.fraction -= -(0x46dc);
  if (SREG & (1 << SREG_C))       // check for carry
    baseaccumulator.integer++;
  baseaccumulator.integer += basestep.integer;

... and the damned carry flag is not set when i expect it to be.

removing the const attribute from the baseoffset declaration fixed things.

now, maybe i can start working on some real bugs.

[mtbf0]

i figured out what's happening.

checked the mega168 datasheet and it turns out that it has subtract immediate instructions, but no add immediate instructions, so if you want to add a constant to a number gcc will replace the addition with a subtraction.

in this example, offsetstep is declared without a const attribute. note that it compiles to 4 load instructions, 2 register to register adds and 2 stores.

Code: Select all

    offsetaccumulator.fraction += offsetstep.fraction;
338:   20 91 1a 01     lds     r18, 0x011A
33c:   30 91 1b 01     lds     r19, 0x011B
340:   80 91 20 01     lds     r24, 0x0120
344:   90 91 21 01     lds     r25, 0x0121
348:   28 0f           add     r18, r24
34a:   39 1f           adc     r19, r25
34c:   30 93 21 01     sts     0x0121, r19
350:   20 93 20 01     sts     0x0120, r18


basestep in this example has been declared with the const attribute. it's value is 0x46dc. instead of adding 0x46dc, gcc decides to subtract 0xb924 using subtract immediate instructions. this eliminates 2 register loads saving 8 bytes of program space and breaks my code.

Code: Select all

    baseaccumulator.fraction += basestep.fraction;
392:   20 91 1d 01     lds     r18, 0x011D
396:   30 91 1e 01     lds     r19, 0x011E
39a:   24 52           subi    r18, 0x24       ; 36
39c:   39 4b           sbci    r19, 0xB9       ; 185
39e:   30 93 1e 01     sts     0x011E, r19
3a2:   20 93 1d 01     sts     0x011D, r18


the result is the same as it would have been if it had compiled to an addition except for the state of the carry flag. which i'm about to check. rats.

removing the const from the basestep declaration gets me this...

Code: Select all

    baseaccumulator.fraction += basestep.fraction;
392:   20 91 1f 01     lds     r18, 0x011F
396:   30 91 20 01     lds     r19, 0x0120
39a:   80 91 11 01     lds     r24, 0x0111
39e:   90 91 12 01     lds     r25, 0x0112
3a2:   28 0f           add     r18, r24
3a4:   39 1f           adc     r19, r25
3a6:   30 93 20 01     sts     0x0120, r19
3aa:   20 93 1f 01     sts     0x011F, r18


and the carry flag gets set when it should and all is right with the world.

[westfw]

I would be VERY surprised if you can check the carry bit of the status register in C with any sort of reliability. This is one the real annoyances of C compared to assembly language; in assembly language, you have access to the 9-bit result of math on 8-bit quantities, and in C you don't. To do this sort of think efficiently, use all assembler. To do it less efficiently, use the next bigger datatype supported by C... IP code is full of checksum algorithms that collect the carries from adding a bunch of 16bit numbers to a 32bit long, for instance; turns out you don't have to add them each time, but only at the end...

[mtbf0]

I would be VERY surprised if you can check the carry bit of the status register in C with any sort of reliability.

Code: Select all

//
// then get next base frequency sample
//
    baseaccumulator.fraction += basestep.fraction;
392:   20 91 1f 01     lds     r18, 0x011F
396:   30 91 20 01     lds     r19, 0x0120
39a:   80 91 11 01     lds     r24, 0x0111
39e:   90 91 12 01     lds     r25, 0x0112
3a2:   28 0f           add     r18, r24
3a4:   39 1f           adc     r19, r25
3a6:   30 93 20 01     sts     0x0120, r19
3aa:   20 93 1f 01     sts     0x011F, r18
    if (SREG & (1 << SREG_C))           // on carry
3ae:   0f b6           in      r0, 0x3f        ; 63
3b0:   00 fe           sbrs    r0, 0
3b2:   05 c0           rjmp    .+10            ; 0x3be <__vector_13+0x194>


Surprise!

checking pages 347-349 of the mega168 datasheet reveals that only two instructions in the above sequence touch the carry flag. guess which ones the are. given that this code executes inside an interrupt service routine and that it's the only interrupt i have enabled, i'd say i'm golden.

i have also disabled the timer0 interrupt that the arduino normally uses for timing, though looking at it in the disassembled listing i see that the status register is saved and restored, so no problem anyway.

the point of the arduino, other than it being a nice affordabe little development platform, is that you hardly have to learn c. i'm old and tired and the last assembler i learned was macro32 and since dec went under my heart's just not in it enough to learn a new one.