WinAVR20080512 optimizer is too good

Go To Last Post
24 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

I want to use a few global register variables (r2, r3, r4) to implement several timers with a single hardware timer:

// System Timer Resolution Unit = ms
#define SYSTEM_TIMER_RESOLUTION     10

#define set_ms_timer(timer, ms)     {timer = ((ms + SYSTEM_TIMER_RESOLUTION / 2) / SYSTEM_TIMER_RESOLUTION);}

register uint8_t isr_temp asm("r2");
register uint8_t shtxx_response_timeout  asm ("r3");
register uint8_t measurement_cycle  asm ("r4");

void init_system_timer (void)
{
    shtxx_response_timeout = 0;
    measurement_cycle = 0;

    // Set Timer1 To Generate Compare Interrupt With The Intervall Time Defined In
    // SYSTEM_TIMER_RESOLUTION
    OCR1A = (F_CPU / 8 + 500 / SYSTEM_TIMER_RESOLUTION) / (1000 / SYSTEM_TIMER_RESOLUTION) - 1;
    
    // Enable Interrupt On Compare A Match
    TIMSK1 = (1 << OCIE1A);

    // Set CTC Mode, Run Timer With F_CPU / 8
    TCCR1A = 0;
    TCCR1B = (1 << WGM12) | (1 << CS11);
}


ISR (TIMER1_COMPA_vect, ISR_NAKED)
{    
    asm volatile ("in %0, %1" : "=r" (isr_temp) : "I" (_SFR_IO_ADDR(SREG)));
    
    // Decrement register variable shtxx_response_ms_timeout If It Is Not Zero
    asm volatile
    (
        "cpse	%0, __zero_reg__" "\n\t"
        "dec    %0"
        : "+r" (shtxx_response_timeout)
        : "0"  (shtxx_response_timeout)
    );
    
    // Decrement register variable measurement_cycle If It Is Not Zero
    asm volatile
    (
        "cpse	%0, __zero_reg__" "\n\t"
        "dec    %0"
        : "+r" (measurement_cycle)
        : "0"  (measurement_cycle)
    );
    
    asm volatile ("out %0, %1" :: "I" (_SFR_IO_ADDR(SREG)), "r" (isr_temp));
    reti();
}

This should enable me to start a software timer this way:

set_ms_timer (measurement_cycle, 100);

And then I can simply poll the register variable if a timeout had occured:

if (measurement_cycle == 0)
{
    // Do Something
}

The problem is that the GCC optimizer detects that the register variable is never changed inside the code (only inside the ISR) and thus deletes the if (measurement_cycle == 0) relevant stuff.

Is there a way to tell the compiler to not delete that code? Of course without using a volatile declared SRAM variable which will blow up my sweet tight ISR.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

S-Sohn wrote:
Hi,

I want to use a few global register variables (r2, r3, r4) to implement several timers with a single hardware timer:

First off, we specifically recommend that you do NOT reserve registers like that. If there is another way to achieve your goal without reserving registers then please do so. When you reserve registers, the compiler and optimizers no longer have access to them. In the case of AVR GCC, I would be unsure of GCC's ability to even cope with that, i.e. the results are unknown and not widely tested and you would probably run into errors.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That's a lot of assembly for a C program, and afaict it only makes the code less readable. Your problem is probably the most frequent issue people get with ISRs; declare your variables volatile.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hm, in which cases does GCC use these registers? It seems to me that GCC doesn't use these registers anyway. Even when using several 32-bit variables inside a function. GCC seems to prefer locating them in SRAM in this case. I already used global register variables in several small projects with success. Is this an issue with the latest WinAVR releases?

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
That's a lot of assembly for a C program
No, only the ISR contains assembly code. Yes, I could have directly stored the ISR in a .S file. The goal of it all is that I can keep the ISR very tight this way (no push/pop). What's wrong with using assembly in a C program?

Quote:
declare your variables volatile.
That wouldn't help in this matter.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK, I lied. The volatile keyword DOES help in this matter. But I was sure I got a warning when declaring a register variable volatile in the past when using older WinAVR releases.

I'm still interested in which cases GCC does use the registers r2, r3, r4.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

From the avr-libc manual:

Quote:
Call-saved registers (r2-r17, r28-r29):
May be allocated by gcc for local data. Calling C subroutines leaves them unchanged.
Assembler subroutines are responsible for saving and restoring these
registers, if changed. r29:r28 (Y pointer) is used as a frame pointer (points to
local data on stack) if necessary. The requirement for the callee to save/preserve
the contents of these registers even applies in situations where the compiler assigns
them for argument passing.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hm, I already read that in the manual. But in my applications there are always some registers which GCC never uses. In my actual application GCC never uses r2, r3, r4, r5 and r6, so I thought I could use them for code optimizations. Maybe my applications aren't complex enough that GCC would use all registers. However, Eric says it's not recommend to use global register variables, so I'll have to look for a workaround. Bummer!
Many thanks!

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
In my actual application GCC never uses r2, r3, r4, r5 and r6,

You SURE about that - do you call any library functions? (and that can include just using the *,/,+,- operators.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, I searched the .lss file to check this. If any library function uses one of my listed registers I should see it, right?

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As long as you read the COMPLETE .lss - both the functions delivered from libc/libm as well as the code you wrote then you should be OK.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is why you shouldn't do this. You are trying to beat the compiler and optimizer and you have to do a lot of double-checking just to make sure that it works. It would be easier on you if you just did not reserve any registers unless for some strange reason your timing requirements need to have such a big hammer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

you have to do a lot of double-checking just to make sure that it works.

And you'd have to do it every time you change something in your code that might cause the compiler to "suddenly" generate code that uses any of those registers you have "claimed".

You could of-course automate this, eg. with a decent grep/awk/whatever-scripting-language-you-like script activated from your makefile, and upon detection of this emit a warning or error like "Warning! My claimed registers have now been claimed by the compiler also - application is no longer flawless!"

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As far as I have noticed until now, GCC seems to prefer the upper registers first and adds the lower registers when doing extensive calculations. GCC seems to do this for each single function. In my application I have finished programming the extensive calculation parts. I only have to add the program flow control. So I thought I'd be on the safe side after checking once which registers are never used. But as you already said: I could be wrong, so I better avoid those tweaks.

Johan, did I understood you correctly? I should add to my makefile a call of a small vb script or .exe which checks the generated .lss file? Nice idea. I'll try it if Eric says it's a safe solution. Maybe there are some more pitfalls.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Johan, did I understood you correctly? I should add to my makefile a call of a small vb script or .exe which checks the generated .lss file? Nice idea.

Uhmmmm.. Well.. No. I rather made that up to demonstrate what risks you are taking here. It would of-course be technically feasible to do it.

Peronally, I'd rather just stay away from the register reservation game and let the compiler do what I ultimately want it to do.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I played around with this feature a while ago.

JohanEkdahl wrote:
I rather made that up to demonstrate what risks you are taking here. It would of-course be technically feasible to do it.

Reading this comment means to me:
    There are (or could be) lib-functions mainly also in assembler that uses fixed registers. These register allocations are not documented. There is a good chance that the compiler ignores my special register allocation.
Normally I would think that the last issue should not come up if I provide my register allocation to every source module. But for the first an extension of the documentation should help - or do I see this too simple?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have finished the workaround. Now I'm using the watchdog interrupt for timings which don't need to be very accurate.

In the watchdog ISR I simply set a flag (single bit located in GPIOR0). This way I can use the ISR_NAKED attribute because the generated code for setting the flag is a SBI instruction which doesn't affect the status register.

Many thanks!

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

S-Sohn wrote:
Quote:
That's a lot of assembly for a C program
No, only the ISR contains assembly code. Yes, I could have directly stored the ISR in a .S file. The goal of it all is that I can keep the ISR very tight this way (no push/pop). What's wrong with using assembly in a C program?
I stand corrected; checking with the compiler, it turns out it generates a superfluous change to r24 for "isr_temp=SREG". The decrement if not 0 worked identically, though. It's not wrong to use assembly, particularly for ISRs such as this, but it's somewhat harder to work with IMHO.

If you're using a simple flag now, why not skip the ISR entirely and use the interrupt flag directly (such as OCF1A)?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
it turns out it generates a superfluous change to r24 for "isr_temp=SREG".
In my application it didn't and it would be fatal if it did. I compiled with -Os. Don't know why the compiler did that in your application.

Quote:
If you're using a simple flag now, why not skip the ISR entirely and use the interrupt flag directly (such as OCF1A)?
The watchdog shall still reset the AVR if the application hangs. So I thought I need an other flag which tells the main loop if the watchdog shall be reset cyclic or not. But you're right all neccessary information are already in the SFRs. The additional LD instruction for reading the WDTCSR should be no problem. Thanks!

Edit: Hm, my first thought was right. I must enable the watchdog interrupt. The watchdog interrupt flag is only set if the watchdog interrupt is enabled (ATmega164P). So polling solution is not possible.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

By the way, this program doesn't use any multiplication opcode? The ATmega164P includes a hardware multiplier, and you are using __zero_reg__ without previous initialization in the ISR(TIMER1_COMPA_vect, ISR_NAKED). Any other library routine can also be using R1 as a temporary scratch register when the program jumps to the ISR.

Regards,
Carlos.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, I'm doing many multiplies in my application. :oops:
I'm becoming more and more thankfull that I followed Eric's advice and canceled my timer method.
Thanks for the hint!

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
The problem is that the GCC optimizer detects that the register variable is never changed inside the code (only inside the ISR) and thus deletes the if (measurement_cycle == 0) relevant stuff.

Jörg Wunsch helped me once a time in the same problem.

volatile register uint16_t s1 asm("r2"); 

ISR(INT0_vect) 
{ 

   if(bit_is_set(PINC,0))    s1++; 
   else            s1--; 
} 

int main(void) { 
//... 
      sei(); 
   OCR0+=10; 
   while(s1<10) 
        asm("":::"r2"); 
//...
}

this code generates

        rjmp .L7 
.L8: 
.L7: 
        ldi r24,lo8(10) 
        cp r2,r24 
        cpc r3,__zero_reg__ 
        brlo .L8

See
https://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=51008&postdays=0&postorder=asc

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A very interesting thread.
Jörg mentioned there that he was sure that you get a warning if you declare a register variable volatile - like me. :D
So my sanity is still there (I was in doubt when I tested LoneTech_ suggestions and I saw no warning).

The empty-asm-statement-trick is great. I'll keep it in my mind.

Regards
Sebastian

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Using register variables sometimes gives significant speedup. gcc uses lower registers very rare. i think it is so because gcc assumes programmer to use this registers. I have no any bad presentiment when i reserve 5 or 6 registers for my needs. Even if you use precompiled libraries you can prevent any damage with saving risk content to RAM.

register volatile int32_t my_variable asm("r2");
volatile int32_t copy_of_my_variable_in_RAM;
...

inline int safe_precompiled function(int foo){
copy_of_my_variable_in_RAM = my_variable;
cli();//if any interrupt uses my_variable
int temp = precompiled function(foo);
my_variable = copy_of_my_variable_in_RAM;
sei();
return temp;
}
...

int main(){
...
int bar = safe_precompiled function(foo);//instead of precompiled function(foo)
...
}