EmbDev.net

Forum: ARM programming with GCC/GNU tools Optimizer broke code - FIXED


von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
This is in reference to the thread "Optimizer broke code".

I rewrote the function due to finding an unrelated bug, because I wanted 
to remove a feature, and because I wanted to make another effort at 
using string functions to simplify it.  Despite the function testing ok, 
once again, in Win32 GCC, it failed in the same way on the ARM chip as 
the previous function.  The new function is below.

To summarize what was occurring, a string passed into the word wrap 
function lcd_DisplayStr is processed, which then calls the LCD display 
function lcd_DspStr.  The string passed into lcd_DspStr is always random 
garbage.

Global -O2 optimization causes lcd_DisplayStr to malfunction.  If I 
compile the entire project with -O2, but only the problematic function 
with "-O2 -fno-forward-propagate", it works.  I looked up what 
forward-propagate is but I don't understand what it's doing.  I guess 
for now I'll just leave it this way.  Oddly enough, this is the third 
version of this function, and the first one works as designed, which 
isn't that drastically different from #2 and #2.

This has been a strange experience.  I'm no C code expert, but I've 
written a fair amount of it for my own stuff, and never ran across a 
problem like this before.

1
int lcd_DisplayStr(unsigned char* instr, int maxlines, int maxwidth, int startaddr, const unsigned short *font, int ellipsis, int invert)
2
{
3
  unsigned char tempc[maxwidth + 1];
4
  unsigned char *strx;
5
6
  int line_count;
7
8
  line_count = 0;
9
10
  while((line_count < maxlines) && (*instr != 0))
11
  {
12
    memset(&tempc[0], 0, (maxwidth + 1));
13
    strncpy(&tempc[0], instr, maxwidth);
14
    strx = strrchr(&tempc[0], ' ');              //look for the last space in output string
15
16
    if((strlen(instr)) <= (strlen(&tempc[0])))        //if the remaining instr will fit into one line, move input string pointer past already displayed text
17
    {
18
      instr = instr + strlen(instr);
19
    }
20
    else if((strx != 0) && ((strx - &tempc[0]) > (maxwidth - FRAGMENT)))  //decide if word fragment long enough to leave on present line 
21
    {
22
      instr = instr + (strx - &tempc[0]);
23
      *strx = 0;                        //terminate output string
24
    }
25
    else
26
    {
27
      instr = instr + maxwidth;                
28
    }
29
30
    while(*instr == ' ')                    //eliminate leading spaces on subsequent output strings
31
    {
32
      instr++;
33
    }
34
35
    while((strlen(&tempc[0])) < maxwidth)          //fill up rest of &tempc[0] line with spaces
36
    {
37
      strcat(&tempc[0], " ");
38
    }
39
40
    line_count++;
41
42
    if((line_count == maxlines) && (*instr != 0) && (ellipsis == 1))  //if entire instr can't be displayed, and ellipsis enabled, display "..."
43
    {
44
      *(&tempc[0] + (maxwidth - 3)) = 0;
45
      strcat(&tempc[0], "...");
46
    }
47
    lcd_DspStr(&tempc[0], startaddr, invert, font);
48
    startaddr = startaddr + (*(font + 1) * LCD_GRAPHIC_LINE);
49
  }
50
  return startaddr;
51
}

von Oliver (Guest)


Rate this post
useful
not useful
Just for my curiosity:

What version of gcc are you using?

The option "-fno-forward-propagate" is not very common on newer gcc's 
(even google does not find it in any gcc-option list). The more recent 
gcc's do know "-f-forward-propagate", but this is enabled in all 
optimization states EXEPT -O2.

Oliver

von Peter S. (psavr)


Rate this post
useful
not useful
Instead of "&tempc[0]" you can use simply "tempc"

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
Peter S. wrote:
> Instead of "&tempc[0]" you can use simply "tempc"

I believe I was doing that before, but thought it was more proper to do 
it how I did.  Thanks for the tip.

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
Oliver wrote:
> Just for my curiosity:
>
> What version of gcc are you using?
>
> The option "-fno-forward-propagate" is not very common on newer gcc's
> (even google does not find it in any gcc-option list). The more recent
> gcc's do know "-f-forward-propagate", but this is enabled in all
> optimization states EXEPT -O2.
>
> Oliver

I'm using CodeSourcery G++ Lite 2011.03-42 (the most recent version), or 
gcc 4.4.1, for ARM.

I found almost 80,000 results for "-fno-forward-propagate" on Google, 
but a little trick; you have to drop the first hyphen, otherwise that 
tells Google to not look for it.  I found that -O2 is enabling this 
optimization by doing "arm-none-eabi-gcc -Q -O2 --help=optimizer".

In any case, adding "-fno-forward-propagate" fixes the problem, but I'd 
love to know why.  Why out of all the other code is it picking on this 
function.

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
I also found that -fforward-propagate is enabled by default on -O, -O2, 
-O3, -Os.

Found on http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

von Oliver (Guest)


Rate this post
useful
not useful
>I also found that -fforward-propagate is enabled by default on -O, -O2,
>-O3, -Os.

Yes, you are right. I mixed up the lines.

Oliver

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
Oliver wrote:
>>I also found that -fforward-propagate is enabled by default on -O, -O2,
>>-O3, -Os.
>
> Yes, you are right. I mixed up the lines.
>
> Oliver

I think this could classify as "the optimizer broke my code", despite 
this generally being very unlikely.

von Oliver (Guest)


Rate this post
useful
not useful
>I think this could classify as "the optimizer broke my code"

Sure. But ist still very likely, that this is caused by your code, and 
very unlikely, that this is caused by the optimizer.

Anyhow, I compiled your code piece with optimization -O2 into a little 
example, and, beside the compiler threw a bunch of warnings related to 
pointer signdness problems, it run flawlessly in the gdb-simulator. 
(Using the yagarto toolchain with gcc 4.5.2).

If you could build a short example, which can be used to reproduce the 
fault, this would be helpfull.

Oliver

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
Oliver wrote:
>>I think this could classify as "the optimizer broke my code"
>
> Sure. But ist still very likely, that this is caused by your code, and
> very unlikely, that this is caused by the optimizer.
>
> Anyhow, I compiled your code piece with optimization -O2 into a little
> example, and, beside the compiler threw a bunch of warnings related to
> pointer signdness problems, it run flawlessly in the gdb-simulator.
> (Using the yagarto toolchain with gcc 4.5.2).
>
> If you could build a short example, which can be used to reproduce the
> fault, this would be helpfull.
>
> Oliver

I could have pointer signedness problems, but my compiler doesn't show 
this, although I do like to eliminate all warnings even if they are 
harmless.  I am getting a warning related to a #define being redefined, 
so warnings are enabled.  The #define problem is an unrelated issue 
involving third party code which I am still tracking down (CPU 
peripheral driver).

Anyway, simply calling the function I posted results in it passing a few 
characters of garbage to lcd_DspStr().  The target does have periodic 
interrupts (but no RTOS) that could be overwriting part of the 
stack...actually recent findings lead me to think this is the problem. 
But no other code is affected, and there's a lot of other things going 
on that should be getting corrupted, and the stack is used heavily.  I 
suppose you would have to have an interrupt in your test to more 
accurately simulate it.

I've never been able to get ARM code simulation to work using gdb.  It 
would be very useful, but I just gave up.  I could put an example 
together, but including the interrupts won't simulate because it's CPU 
dependent, correct?  I'm just not sure how to provide an example that 
can be simulated without a lot of work on your side.

von Oliver (Guest)


Rate this post
useful
not useful
gdb does not simulate Interrups, or any other ARM-hardware. To debug 
this you need a JTAG debugger for the real hardware, or a commercial 
siumulator. With this it should not be a big problem to find the point 
in the code where tempc[] gets screwed up.

You will get the warnings, if you compile with -Wall, which is a good 
idea to use as the standard warning flag. Just to see, what else could 
be suspicous, add -Wextra, and if you ever plan to write standard-C, try 
-pedantic (although this is not recommeded even by the gcc authors).

Oliver

von Klaus W. (mfgkw)


Rate this post
useful
not useful
Do you really think -O3 could help to improve this?

von jrmymllr j. (jrmymllr)


Rate this post
useful
not useful
Klaus Wachtler wrote:
> Do you really think -O3 could help to improve this?

I don't think so, and I think the same option causing me problems is 
enabled with -O3 also.

Awhile ago I did some comparisons between the different optimization 
levels, and found that -O3 resulted in larger code, and if I recall 
correctly, worse or the same performance.  So I stuck with -O2 in this 
case.

Please log in before posting. Registration is free and takes only a minute.
Existing account
Do you have a Google/GoogleMail account? No registration required!
Log in with Google account
No account? Register here.