public inbox for ecos-discuss@sourceware.org
 help / color / mirror / Atom feed
From: Doug Fraser <dfraser@photuris.com>
To: 'Gary Thomas' <gthomas@redhat.com>, rob.wj.jansen@philips.com
Cc: ecos-discuss@sourceware.cygnus.com, tadams@extremeeng.com
Subject: RE: [ECOS] Optimizations and bad code
Date: Mon, 23 Jul 2001 09:47:00 -0000	[thread overview]
Message-ID: <E2D27064CD59574F88D05AEF5728396D4453FB@PH01SRV02.photuris.com> (raw)

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 :-(
> 

             reply	other threads:[~2001-07-23  9:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-07-23  9:47 Doug Fraser [this message]
2001-07-23 12:36 ` Jonathan Larmour
     [not found] <E2D27064CD59574F88D05AEF5728396D4453FD@PH01SRV02.photuris.com>
2001-07-24  6:23 ` Jonathan Larmour
  -- strict thread matches above, loose matches on Subject: below --
2001-07-24  1:07 rob.wj.jansen
2001-07-23  0:58 rob.wj.jansen
2001-07-23  6:32 ` Gary Thomas
2001-07-23  9:31   ` Trenton D. Adams
2001-07-20 10:31 Trenton D. Adams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E2D27064CD59574F88D05AEF5728396D4453FB@PH01SRV02.photuris.com \
    --to=dfraser@photuris.com \
    --cc=ecos-discuss@sourceware.cygnus.com \
    --cc=gthomas@redhat.com \
    --cc=rob.wj.jansen@philips.com \
    --cc=tadams@extremeeng.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).