Re: [avr-libc-dev] [patch #7909] Adding __volatile__ to __asm__ within pgmspace header

[David Brown]

On 31/12/12 06:54, Alex Crawford wrote:
> URL:
>    <http://savannah.nongnu.org/patch/?7909>
>
>                   Summary: Adding __volatile__ to __asm__ within pgmspace
> header
>                   Project: AVR C Runtime Library
>              Submitted by: crawford
>              Submitted on: Mon 31 Dec 2012 05:54:17 AM GMT
>                  Category: None
>                  Priority: 5 - Normal
>                    Status: None
>                   Privacy: Public
>               Assigned to: None
>          Originator Email:
>               Open/Closed: Open
>           Discussion Lock: Any
>
>      _______________________________________________________
>
> Details:
>
> I noticed that gcc was optimizing out calls to pgm_read_word_far() that were
> within a while loop. Adding the volatile directive fixes the underlying
> problem. I've added the directive to each of the LPM and ELPM calls (20 in
> all).
>
> The patch was generated from avr-libc-1.8.0.
>
> -Alex

Could you post an example of the original C code and generated assembly 
code demonstrating that there is actually a problem?  I would very much 
hope that the compiler will optimise out calls to read program memory 
within a loop if it does not have to repeat them.  Program memory is not 
volatile (baring the unusual case of re-programming), and forcing the 
compiler to re-read data is a waste of time and space.  On the other 
hand, if the compiler is making an illegal optimisation here then 
perhaps this patch is just hiding a bigger problem.

mvh.,

David




>      _______________________________________________________
>
> File Attachments:
>
>
> -------------------------------------------------------
> Date: Mon 31 Dec 2012 05:54:17 AM GMT  Name: volatile.patch  Size: 13kB   By:
> crawford
> Patch for avr/pgmspace.h
> <http://savannah.nongnu.org/patch/download.php?file_id=27168>
>
>      _______________________________________________________
>
> Reply to this item at:
>
>    <http://savannah.nongnu.org/patch/?7909>
>
> _______________________________________________
>    Message sent via/by Savannah
>    http://savannah.nongnu.org/
>
>
> _______________________________________________
> AVR-libc-dev mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/avr-libc-dev