RE: [ECOS] Optimizations and bad code

RE: [ECOS] Optimizations and bad code

[Doug Fraser]

Yup, it is ugly code, but because 'i' is used in the context
of an argument to a function, the optimizer must honor the
scope of the variable within the for() loop. The value of
'i' after each func() call had better be three greater than
before. Its value within the array subscipts is definitely
undefined. That the arm-elf compiler only increments 'i' by
one for a particular optimizer setting is a defect, and badly
formed source code is no excuse for erroneous object code.
If that held true, where would the obsfucated C contest go?

As an additional exercise to the arm compiler, I would try this...

	int i, j;

	i = 0;
	for(j = 0; j < 100; j++)
	{
		i++, i++, i++;     /* a perfectly legit 'comma' operator */
		printf("i %d, j %d\n", i, j);
	}

while j is incremented by one on each print, i must increment by three......

Doug

> -----Original Message-----
> From: Gary Thomas [ mailto:gthomas@redhat.com ]
> Sent: Monday, July 23, 2001 9:32 AM
> To: rob.wj.jansen@philips.com
> Cc: ecos-discuss@sourceware.cygnus.com; tadams@extremeeng.com
> Subject: Re: [ECOS] Optimizations and bad code
> 
> 
> 
> On 23-Jul-2001 rob.wj.jansen@philips.com wrote:
> > 
> > Compiler optimizations can be a real pain, especially if 
> you have some funny code.
> > Look at the following example:
> > 
> > {
> >     int i, y;
> > 
> >    i = 0;
> > 
> >     for(y=0; y<176; y++)
> >     {
> >         func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
> >     }
> > }
> > 
> > This is real ugly code, extracted from a non working progam.
> > It was meant to combine three values from an RGB buffer 
> (containing a 24 bit image)
> > into one 8 bit gray scale value. This was no serious 
> programming but just a fast hack
> > to get something on the LCD.
> > 
> > According to the C standard, the arguments to the + 
> operator may be evaluated in any
> > order, making it unpredictable to tell if the R, G or B 
> value will get the <<1. But I guess that
> > the "i" should always be three more that it started of with 
> after going through the loop once.
> > 
> > The amizing part however is that with the arm-elf-gcc 
> compiler, it depends if optimizations
> > are on "-O2" or off "-O0". Without optimizations i is 
> incremented with 1 (!) after going through
> > the loop once. All other variants (including different 
> optimization settings for the
> > i686-pc-linux-gnu
> > and native linux compiler) have i incremented by 3.
> > 
> > Although I admit that one should never write code like 
> this, the results are amazing.
> > It just shows that "working in my private enviroment" does 
> not mean it is correct code.
> 
> This code is simply frightening!!
> 
> Firstly, in the line
>          func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
> the order of the i++ is not guaranteed at all, so the results 
> could vary
> widely from platform/architecture to platform.
> 
> Also, since the value of 'i' is not used after the for{} 
> loop, or even within it except
> as an index, there is no reason at all for the compiler to 
> ever keep it around or make
> the updated value available.
> 
> The worst part of it all is that even "This was no serious 
> programming but just a fast hack",
> writing this sort of code can become habitual.  Once in that 
> "mold", the programmer will
> write similar code for "serious" use and then wonder why it 
> doesn't work :-(
> 

Re: [ECOS] Optimizations and bad code

[Jonathan Larmour]

Doug Fraser wrote:
> That the arm-elf compiler only increments 'i' by
> one for a particular optimizer setting is a defect, and badly
> formed source code is no excuse for erroneous object code.

Not true at all. Badly formed source code is a perfectly valid reason to
produce code that doesn't do what the programmer expects. After all in
situations like this, how can the compiler guess what the programmer
expects. Maybe it should warn though - I don't know if it does (at default
level or at -Wall).

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine
Come to the Red Hat TechWorld open source conference in Brussels!
Keynotes, techie talks and exhibitions    http://www.redhat-techworld.com/

Re: [ECOS] Optimizations and bad code

[Jonathan Larmour]

Hi Doug,

Doug Fraser wrote:
> When I made the statement about the compiler emitting
> erroneous object code, I was only alluding to the
> fact that the C line in question contained three
> distinct post-increment operators, and the author
> of the original message claimed that after that line,
> the variable had only incremented by one. That is
> an error, as I illustrated in my ill formed response,
> even a simple comma operator should be able to handle
> three post-increments properly. Certainly, the time at
> which each individual increment is performed is
> undefined. All that matters is that they all complete
> before the closure, which is the semicolon that terminates
> both the evil source line being discussed or the simple
> comma separated triplett in my first reply. I quess
> ANSI C presriptivists would call that a 'checkpoint'.

The standard uses the term "sequence point" :).

You assume that post-increments are in some way cumulative. That isn't
necessarily so. The comma operator is different because it is explicitly
defined that the comma is a sequence point. But if you pretend for a moment
that it isn't, then you can say that i++ is the same as (i=i+1,i-1).

So
a[i++] + (a[i++]<<1) + a[i++]

is the same as:

a[(i=i+1,i-1)] + a[(i=i+1,i-1)]<<1) + a[(i=i+1,i-1)];

with the only sequence point being at the end. Written out like this you
can hopefully see now that with multiple side effects like this the problem
is "when do you read i". It is only at sequence points that "all side
effects of previous evaluations shall be complete and no side effects of
subsequent evaluations shall have taken place" (according to the standard).
Because the sequence point is at the end, the value of i between the
previous sequence point and this sequence point can be anything from i to
i+3. And in the absence of any further sequence points, it's not defined
which.

And when using the comma operator (for real), there is a sequence point
each time, so that's different.

> You are right though, that badly formed code is a bad thing.
> It trips up developers when the compiler does something 'odd',
> and worse, it trips up maintainers that follow them later.

The classic way to make mistakes like the above is to pass in an expression
with side effects to a macro, and that can be done virtually undetectably.
I would hope the compiler would warn.
 
> My favorite quote on code complexity came from a colleague
> some years ago when I worked at Bell Labs. We were having
> a discussion at a code review on how one particular module
> could be greatly simplified, and her response was:
> 
> "Then anybody off the street could understand it."
> 
> The team leader replied, "Exactly!"

Heh. If anybody could understand it we'd be out of a job ;-).

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine
Come to the Red Hat TechWorld open source conference in Brussels!
Keynotes, techie talks and exhibitions    http://www.redhat-techworld.com/

Re: [ECOS] Optimizations and bad code

[rob.wj.jansen]

Hi,

I completely agree with Gary,

> This code is simply frightening!!

so the example I made obviously was not typed in exactly like this, this was the
simple version of the macro infested source code I started off with.
The a[i++] was part of a macro (like getc() is a macro in a number of stdio.h files)
where it fetches a byte from a buffer.

Well, they warned me about this kind of stuff with macros ...
That's why I'm so glad that insure++ is available for Linux ;-)

Regards,

     Rob Jansen

Software Engineer
Competence Center Platforms
BU Mobile Communications
Meijhorst 60-10, 6537 KT Nijmegen, The Netherlands
Tel: +31-24-353-6329
Fax: +31-24-353-3613
mailto:Rob.WJ.Jansen@philips.com

Re: [ECOS] Optimizations and bad code

[rob.wj.jansen]

Compiler optimizations can be a real pain, especially if you have some funny code.
Look at the following example:

{
    int i, y;

   i = 0;

    for(y=0; y<176; y++)
    {
        func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
    }
}

This is real ugly code, extracted from a non working progam.
It was meant to combine three values from an RGB buffer (containing a 24 bit image)
into one 8 bit gray scale value. This was no serious programming but just a fast hack
to get something on the LCD.

According to the C standard, the arguments to the + operator may be evaluated in any
order, making it unpredictable to tell if the R, G or B value will get the <<1. But I guess that
the "i" should always be three more that it started of with after going through the loop once.

The amizing part however is that with the arm-elf-gcc compiler, it depends if optimizations
are on "-O2" or off "-O0". Without optimizations i is incremented with 1 (!) after going through
the loop once. All other variants (including different optimization settings for the i686-pc-linux-gnu
and native linux compiler) have i incremented by 3.

Although I admit that one should never write code like this, the results are amazing.
It just shows that "working in my private enviroment" does not mean it is correct code.

Regards,

     Rob Jansen

Software Engineer
Competence Center Platforms
BU Mobile Communications
Meijhorst 60-10, 6537 KT Nijmegen, The Netherlands
Tel: +31-24-353-6329
Fax: +31-24-353-3613
mailto:Rob.WJ.Jansen@philips.com

Re: [ECOS] Optimizations and bad code

[Gary Thomas]

On 23-Jul-2001 rob.wj.jansen@philips.com wrote:
> 
> Compiler optimizations can be a real pain, especially if you have some funny code.
> Look at the following example:
> 
> {
>     int i, y;
> 
>    i = 0;
> 
>     for(y=0; y<176; y++)
>     {
>         func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
>     }
> }
> 
> This is real ugly code, extracted from a non working progam.
> It was meant to combine three values from an RGB buffer (containing a 24 bit image)
> into one 8 bit gray scale value. This was no serious programming but just a fast hack
> to get something on the LCD.
> 
> According to the C standard, the arguments to the + operator may be evaluated in any
> order, making it unpredictable to tell if the R, G or B value will get the <<1. But I guess that
> the "i" should always be three more that it started of with after going through the loop once.
> 
> The amizing part however is that with the arm-elf-gcc compiler, it depends if optimizations
> are on "-O2" or off "-O0". Without optimizations i is incremented with 1 (!) after going through
> the loop once. All other variants (including different optimization settings for the
> i686-pc-linux-gnu
> and native linux compiler) have i incremented by 3.
> 
> Although I admit that one should never write code like this, the results are amazing.
> It just shows that "working in my private enviroment" does not mean it is correct code.

This code is simply frightening!!

Firstly, in the line
         func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
the order of the i++ is not guaranteed at all, so the results could vary
widely from platform/architecture to platform.

Also, since the value of 'i' is not used after the for{} loop, or even within it except
as an index, there is no reason at all for the compiler to ever keep it around or make
the updated value available.

The worst part of it all is that even "This was no serious programming but just a fast hack",
writing this sort of code can become habitual.  Once in that "mold", the programmer will
write similar code for "serious" use and then wonder why it doesn't work :-(

RE: [ECOS] Optimizations and bad code

[Trenton D. Adams]

  > 
  > 
  > On 23-Jul-2001 rob.wj.jansen@philips.com wrote:
  > >
  > > Compiler optimizations can be a real pain, especially if you have
some
  > funny code.
  > > Look at the following example:
  > >
  > > {
  > >     int i, y;
  > >
  > >    i = 0;
  > >
  > >     for(y=0; y<176; y++)
  > >     {
  > >         func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
  > >     }
  > > }
  > >
  > > This is real ugly code, extracted from a non working progam.
  > > It was meant to combine three values from an RGB buffer
(containing a
  > 24 bit image)
  > > into one 8 bit gray scale value. This was no serious programming
but
  > just a fast hack
  > > to get something on the LCD.
  > >
  > > According to the C standard, the arguments to the + operator may
be
  > evaluated in any
  > > order, making it unpredictable to tell if the R, G or B value will
get
  > the <<1. But I guess that
  > > the "i" should always be three more that it started of with after
  > going through the loop once.
  > >
  > > The amizing part however is that with the arm-elf-gcc compiler, it
  > depends if optimizations
  > > are on "-O2" or off "-O0". Without optimizations i is incremented
with
  > 1 (!) after going through
  > > the loop once. All other variants (including different
optimization
  > settings for the
  > > i686-pc-linux-gnu
  > > and native linux compiler) have i incremented by 3.
  > >
  > > Although I admit that one should never write code like this, the
  > results are amazing.
  > > It just shows that "working in my private enviroment" does not
mean it
  > is correct code.
  > 
  > This code is simply frightening!!
  > 
  > Firstly, in the line
  >          func(x, y, (a[i++] + (a[i++]<<1) + a[i++])>>2);
  > the order of the i++ is not guaranteed at all, so the results could
vary
  > widely from platform/architecture to platform.
  > 
  > Also, since the value of 'i' is not used after the for{} loop, or
even
  > within it except
  > as an index, there is no reason at all for the compiler to ever keep
it
  > around or make
  > the updated value available.
  > 
  > The worst part of it all is that even "This was no serious
programming
  > but just a fast hack",
  > writing this sort of code can become habitual.  Once in that "mold",
the
  > programmer will
  > write similar code for "serious" use and then wonder why it doesn't
work
  > :-(

Happy to say that I don't have anything remotely that NASTY in my code,
nor will there ever be any!  So, it's unlikely it's optimizations.
Besides, I already disabled optimizations in the makefile for my PCMCIA
driver, and that had no effect.  Still can't read the piece of crap CIS
though.  Cirrus sucks for support.

[ECOS] Optimizations and bad code

[Trenton D. Adams]

I was thinking.  Maybe my PCMCIA controller driver isn't working
properly because the compiler optimizations are screwing it up.  Is this
possible?  If so, how do I turn them off just for edb7xxx_pcmcia.c?

I tried turning them off completely, and an eCos application will not
run whatsoever.  And yes, I did do a Build|Clean first.

Trenton D. Adams
Embedded Developer
Windows Developer
Extreme Engineering Ltd.
Calgary Alberta.