Re: Structure vs purism ?

Mark Phillips (msp@nortelnetworks.com)
Fri, 22 Jan 1999 14:23:13 +0000 (GMT)


Correction:-

On Thu, 21 Jan 1999, Richard B. Johnson wrote:

> On Thu, 21 Jan 1999, Philipp Rumpf wrote:
>
> > On Wed, Jan 20, 1999 at 09:58:53PM -0500, Richard B. Johnson wrote:
> > > Then 'tidy' up something so it looks good to 'humans' and do the same
> > > thing again. You will then learn why there are strange constructs and,
> > > as you say, a 1000 gotos.
> >
> > I think a lot of the goto's could be removed as soon as there is a decent
> > possibility to tell the compiler to optimize for a certain case. This was
> > discussed on the egcs lists some time ago IIRC.
> >
>
> Maybe, one function at a time --one carefully checked function at a time.
> For instance, the way we were 'taught` to write code (by instuctors and
> professors who never built actual products, BTW) goes something like
> this...
>
> funct()
> {
> if(funct1()) {
> if(funct2()) {
> if(funct3()) {
> if(funct4())
> do_something();
> else
> compain_about_funct4();
> }
> else
> complain_about_funct3();
> }
> else
> complain_about_funct2();
> }
> else
> complain_about_funct1();
> }
> }
>
> ...... produces absolutely rotten code on every compiler I've ever
> used. This is because it jumps on the normal-flow condition.

The problem with this argument is that it is very easy to get it wrong....

The above code generates asm structured as follows:-

if(!func1()) goto func1_bad;
if(!func2()) goto func2_bad;
if(!func3()) goto func3_bad;
if(!func4()) goto func4_bad;
do_something();
goto done;

func4_bad:
complain_about_funct4();
goto done;

func3_bad:
complain_about_funct3();
goto done;

func2_bad:
complain_about_funct2();
goto done;

func1_bad:
complain_about_funct1();

done:
ret

// Above was checked against egcs 1.1.1 without optimsation and with -O2.

Which is basically identical to your hand coded goto code!

On the other hand I must admit that I don't like the style of the above
code - I find it hard to read and error prone (wrong # or } etc).

Unfortunately the style often used by people (including me:-) does have
the problem you are describing:-

func()
{
if(!funct1(){
complain_about_funct1();
return error_c;
}

if(!funct2(){
complain_about_funct2();
return error_c;
}
...

Generates asm code structured in the same way as the C.

In particular if complain_about_functX() is replaced with actual recovery
code, or by a function with lots of arguments, you will end up cluttering
the pipe with code which is unlikely to get executed. So moving the
recovery to a function is good. Of course if the recovery requires local
vars then a function will not work, so a goto works better (or using C++,
if the vars required by the recovery are member vars, because then a
recovery member function can see them...).

> This jumps only on the error conditions.
>
> Sometimes gotos can be removed as:
>
> if(!funct1())
> return BAD;

Yep!, and I agree with your reservations - error recovery code SOMETIMES
can be cleaner and faster using goto. This also applies to recovering from
a recursive process which fails.

>
> Even the usual loop-counters often produce bad, bad, bad code.
>
> for(i=0; i< MAX; i++)
> do_something();
>
> You would think that, since this construct is so often used, the 'C'
> compilers would at least optimize this. Nope, guess again. Even telling
> the compiler to use 'register int i`, even if it does, the loop test
> and the increment end up being in the wrong order, forcing extra code

Be careful, this depends on your compiler.... Therefore writing obscure
code to bodge around the problem is not good.

On the other hand:-

i= MAX;
do{
do_something();
}while(i--);

Is clear and makes life easier for the compiler.

BUT note that egcs 1.1.1 does not generate code as bad as you expect. It
does still use an increment loop, BUT it does realise that the loop will
always be executed at least once so only emit the test at the end of the
loop (using -O2, looking at sparc - because I'm at work...).

>
> void foo()
> {
> puts("Do not optimize this away!");
> }
> main()
> {
> register int i;
> i = 10;
> while(i--)
> foo();
> }
>

The resulting mess looks like compiler bugs which should be reported and
fixed!

The resulting code for a similar example with egcs 1.1.1 -O2 on a sparc
is MUCH cleaner!

Keep in mind the sparc is a RISC chip.

78:gen2.c ****
79:gen2.c **** i=10;
80:gen2.c **** while(i--){
398 .stabn 68,0,80,.LM35-loop2
399 .LM35:
400 0194 A0102009 mov 9,%l0
401 .LL27:
81:gen2.c **** f();
402 .stabn 68,0,81,.LM36-loop2
403 .LM36:
404 0198 40000000 call f,0
405 019c A0043FFF add %l0,-1,%l0
82:gen2.c **** }
406 .stabn 68,0,82,.LM37-loop2
407 .LM37:
408 01a0 80A43FFF cmp %l0,-1
409 01a4 12BFFFFD bne .LL27
410 01a8 01000000 nop
83:gen2.c ****
84:gen2.c **** }
411 .stabn 68,0,84,.LM38-loop2
412 .LM38:
413 .LLBE3:
414 01ac 81C7E008 ret
415 01b0 81E80000 restore

>
> So, to have a fast Operating System, one must often compromise asthetics
> and get down and dirty, mucking with the registers.

Yes, but I worry that people are too keen to do this at the expense of
unmaintainable code and/or code which is crap on poor on other platforms.

Maybe you should join the egcs developers?! :-)

Cheers
Mark
>

-- 
Mark S. Phillips        ESN 742 2461
msp@nortel.co.uk        Tel. +44 1279 402461

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/