2
\$\begingroup\$

I tried to compile the following C code:

period = TCNT0L;
period |= ((unsigned int)TCNT0H<<8);

The assembler code I'm getting is the following:

    period = TCNT0L;
  d2:   22 b7           in  r18, 0x32   ; 50
  d4:   30 e0           ldi r19, 0x00   ; 0
  d6:   30 93 87 00     sts 0x0087, r19
  da:   20 93 86 00     sts 0x0086, r18
    period |= ((unsigned int)TCNT0H<<8);
  de:   44 b3           in  r20, 0x14   ; 20
  e0:   94 2f           mov r25, r20
  e2:   80 e0           ldi r24, 0x00   ; 0
  e4:   82 2b           or  r24, r18
  e6:   93 2b           or  r25, r19
  e8:   90 93 87 00     sts 0x0087, r25
  ec:   80 93 86 00     sts 0x0086, r24

So instead of 4 instructions it gets 11!

I tried to choose O1, O2, O3 and Os optimization options. The result is the same (except that O3 option optimized away this code at all).

I could write the source code in the following way:

period = TCNT0L | ((unsigned int)TCNT0H<<8);

I will get smaller, but still not optimal code:

  de:   22 b7           in  r18, 0x32   ; 50
  e0:   34 b3           in  r19, 0x14   ; 20
  e2:   93 2f           mov r25, r19
  e4:   80 e0           ldi r24, 0x00   ; 0
  e6:   82 2b           or  r24, r18
  e8:   90 93 87 00     sts 0x0087, r25
  ec:   80 93 86 00     sts 0x0086, r24

However I will not have a guaranty that the lower byte will be accessed first any more (this is essential requirement to keep 16-bit reading correct). And still the code has many extra unnecessary instructions.

Am I able to change compiler options and/or change the source code to make it better? I'd avoid go to assembler.

UPDATE1:

I tried the code @caveman suggested:

((unsigned char*)(&period))[0] = TCNT0L;
((unsigned char*)(&period))[1] = TCNT0H;

But the result is also not very good:

    ((unsigned char*)(&period))[0] = TCNT0L;
  dc:   82 b7           in  r24, 0x32   ; 50
  de:   e6 e8           ldi r30, 0x86   ; 134
  e0:   f0 e0           ldi r31, 0x00   ; 0
  e2:   80 83           st  Z, r24
    ((unsigned char*)(&period))[1] = TCNT0H;
  e4:   84 b3           in  r24, 0x14   ; 20
  e6:   81 83           std Z+1, r24    ; 0x01
\$\endgroup\$
8
  • 3
    \$\begingroup\$ Can't you just do: uint8_t period = TCNT0 ? \$\endgroup\$
    – Golaž
    Feb 10 '15 at 11:37
  • \$\begingroup\$ @Golaž this is weird but my compiler does not accept TCNT0 from the shelf. Do I need to declare it by my self? If yes - how do I declare 16-bit registers (taking into account the fact that the processor is 8 bit) \$\endgroup\$ Feb 10 '15 at 11:40
  • \$\begingroup\$ Which IDE are you using? \$\endgroup\$
    – Golaž
    Feb 10 '15 at 11:41
  • \$\begingroup\$ @Golaž Atmel Studio 6 \$\endgroup\$ Feb 10 '15 at 11:43
  • 1
    \$\begingroup\$ @PeterJ 8bit AVR provide high bite lock after low half of 16-bit register reading \$\endgroup\$ Feb 10 '15 at 11:50
3
\$\begingroup\$

One method is to use direct loads to the halves of period. While this looks complicated in C, it usually will generate very tight assembly, i.e. 2 loads and 2 stores.

((uint8_t*)(&period))[0] = TCNT0L;
((uint8_t*)(&period))[1] = TCNT0H;

Sometimes using the array math can cause issues so you could try this:

*((uint8_t*)(&period)) = TCNT0L;
*((uint8_t*)(&period) + 1) = TCNT0H;

This actually produces optimal code. Look at how there are 12 bytes used.

  ((unsigned char*)(&period))[0] = TCNT0L;
  dc:   82 b7           in  r24, 0x32   ; 50
  de:   e6 e8           ldi r30, 0x86   ; 134
  e0:   f0 e0           ldi r31, 0x00   ; 0
  e2:   80 83           st  Z, r24
    ((unsigned char*)(&period))[1] = TCNT0H;
  e4:   84 b3           in  r24, 0x14   ; 20
  e6:   81 83           std Z+1, r24    ; 0x01

If you did this with assembly, it would probably seem better to do it like this. It is also 12 bytes, so they are equivalent.

  dc:   82 b7           in  r24, 0x32   ; 50
  de:   80 93 86 00     sts 0x0086, r24
  e2:   84 b3           in  r24, 0x14   ; 20
  e4:   80 93 87 00     sts 0x0087, r24

Of course, when I say "equivalent", I mean regarding code size. If time is more important, then you have to look at the cycles. In this case it looks like the assembly version is 6 cycles and the compiler's version is 8 cycles.

\$\endgroup\$
7
  • \$\begingroup\$ Tried your code. Worked partially. 7 instruction which is equal to one-operator approach :( (see my question UPDATE1) \$\endgroup\$ Feb 10 '15 at 12:24
  • \$\begingroup\$ You can try using this non-array math version. But, frankly, just use inline assembly. The code generator appears to be pretty bad on this port of gcc. \$\endgroup\$
    – caveman
    Feb 10 '15 at 12:41
  • \$\begingroup\$ Tried the second: same result. Regarding R30/R31 - looks like it tries to use indirect storing (as I've used indirect addressing method in the code) so it looks pretty warrantable. \$\endgroup\$ Feb 10 '15 at 12:46
  • \$\begingroup\$ Just noticed your last edit: my main concern is execution time. So assembler version is the thing I really need. So can I assume that this code can not be correctly obtained from C compiler (without using inline assembler)? \$\endgroup\$ Feb 10 '15 at 20:46
  • \$\begingroup\$ I don't think the compiler will be able to do much better. You have tried a lot of things. But let me ask if you have stepped back and really think 2 cycles is worth it? If all of your code is that tight, you probably should be using assembly directly anyways. \$\endgroup\$
    – caveman
    Feb 10 '15 at 22:52
0
\$\begingroup\$

In my avr-gcc 5.4.0 simple period = TCNT1; for attiny841 seems to emit the code like this:

    in  r24,0x2c
    in  r25,0x2d
    sts 0x0110,r25
    sts 0x010f,r24

It seems that the compiler knows already of the way 16-bit registers must be accessed and therefore the code like above is safe.

Avr branch of the gcc generally is not very good even in simple optimizations like the examples in the question, bun anyway upgrading the version of avr-gcc often helps.

Another concern is that later gccs and later avr-libcs could actually support accessing TCNT0 as single 16-bit register -- what seems to lack in the gcc used in the question.

\$\endgroup\$
2
  • \$\begingroup\$ Ahhh! I hate every time I see this motif emitted! Why does the compiler always waste an extra register rather than doing the equivalent.. ` in r24,0x2c sts 0x0110,r24 in r24,0x2d sts 0x010f,r24 ` ...!? \$\endgroup\$
    – bigjosh
    Nov 12 '18 at 21:33
  • 1
    \$\begingroup\$ @bigjosh Especially painful when all you want is to increment a uint32_t (or longer) in an ISR. In these cases, the compiler sometimes could get away with 0 registers (use the temp register!) but pushes four of them to the stack and pops them back on return. I have an asm macro ready for those special cases. \$\endgroup\$
    – JimmyB
    Nov 13 '18 at 13:45
0
\$\begingroup\$

If you are willing to waste a pin, you could get a 1-instruction/2-cycle capture of the TCNT when the ISR is called by using the Output Capture Unit.

Setup

  1. Set bit in the DDR for ICP pin to make it an output.
  2. Set ACIC to use the input pin for ICU trigger. Leave other ICU bits to defaults (no noise filter, trigger on falling edge)

For each capture

In foreground

  1. Clear the ICF bit by writing a 1 to it.
  2. Set the PORT bit for the ICP pin to make it output HIGH.
  3. Poll on the ICF bit until it turns to 1.
  4. Read captured TNCT value out of ICR register.
  5. Rinse. Repeat.

In ISR

  1. Set PORT bit for ICP pin using the SBI instruction.

enter image description here

enter image description here

enter image description here

\$\endgroup\$
0
\$\begingroup\$

If you want to go hard-core with cycle savings, you could get this grab of TCNT down to a single cycle in the ISR!

You can take advantage of the fact the high byte of the TCNT register is buffered whenever the low byte is read.

enter image description here

So if you preallocated a register (say r16) for this task...

register unsigned char tcnt_low_byte asm("r16");

...then filled this register with the low byte of the TCNT inside the ISR like this...

R16 = TCNTL;

...which should compile down to the 1-cycle...

IN R16,TCNTL

...then you could later read out the full snapshotted TCNT value int he foreground like this....

period = (TCNTH << 8)| R16;

Just make sure you read the TCNTH before accessing any other 16 bit timer registers since all of them share that temp temp register.

The total work done in the ISR is just a single in R16, TCNTL which is 1 cycle.

The OP did not indicate how he would signal the foreground process that an ISR happened, but if he was preloading period with 0 and then looking for a change then some extra work is needed...

  1. preload 0 into the TEMP 16-bit register (you can do this by writing a 0 to any 16 bit register).
  2. preload 0 into R16.

Then you can poll to see if the ISR happened with...

x=TCNTH
if (x || R16) {
    period=(x<<8 | R16)
    // Process new period capture here...
}
\$\endgroup\$
3
  • \$\begingroup\$ Interesting idea. But how would he communicate from the ISR to the main loop that TCNTL has been read and it's time to read TCNTH? \$\endgroup\$
    – JimmyB
    Nov 12 '18 at 12:41
  • \$\begingroup\$ @JimmyB Great question, I had not considered that! Presumably using whatever mechanism the OP used to signal that the TCNT value had been captured into period in his example (not shown)? Answer updated for the case if his mechanism was to preset period to 0 and then poll it. \$\endgroup\$
    – bigjosh
    Nov 12 '18 at 18:43
  • \$\begingroup\$ @JimmyB See any problems? I think the access to x+R16 does not even need to be atomic. \$\endgroup\$
    – bigjosh
    Nov 12 '18 at 18:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.