Re: Patch to remove undefined C code

2000-10-20 Thread Albert D. Cahalan

> Yes.  In practice the usual question is whether the compiler will
> evaluate the operands from left to right or from right to left,
> but the compiler is within its rights to evaluate the operands in
> any order it wants.

For ia64, it would be good to evaluate the operands in parallel.
One could do the same with naked Transmeta hardware and other VLIW
processors.

Tera's machine has cheap hardware threads that could be used.
Compaq's 21364 or 21464 may allow this too.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-20 Thread Rick Hohensee

([EMAIL PROTECTED]) nobly encouraged empiricism thusly...
>   Just to encourage empiricism, I usually check stuff like that with
>
>   % cat > foo.c
>   main(){
>   int i;
>   i = 1 , 2;
>   printf("%d\n",i);
>   }
>   % gcc foo.c
>   % ./a.out
>   1
>
>   or similar if I'm confused.

Indeed. Further curiosity is sometimes promptly slaked in a manner
such as this...


:; cLIeNUX0 /dev/tty4  17:43:31   /
:;cc1
main(){
int i;
i = 1 , 2;
printf("%d\n",i);
}
:; cLIeNUX0 /dev/tty4  17:43:31   /
:;

The output of cc1 is not shown in the above. You'll have to
play that little ode to the joy of empiricism yourself.

Rick Hohensee
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-20 Thread Rick Hohensee

([EMAIL PROTECTED]) nobly encouraged empiricism thusly...
   Just to encourage empiricism, I usually check stuff like that with

   % cat  foo.c
   main(){
   int i;
   i = 1 , 2;
   printf("%d\n",i);
   }
   % gcc foo.c
   % ./a.out
   1

   or similar if I'm confused.

Indeed. Further curiosity is sometimes promptly slaked in a manner
such as this...


:; cLIeNUX0 /dev/tty4  17:43:31   /
:;cc1
main(){
int i;
i = 1 , 2;
printf("%d\n",i);
}
:; cLIeNUX0 /dev/tty4  17:43:31   /
:;

The output of cc1 is not shown in the above. You'll have to
play that little ode to the joy of empiricism yourself.

Rick Hohensee
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-20 Thread Albert D. Cahalan

 Yes.  In practice the usual question is whether the compiler will
 evaluate the operands from left to right or from right to left,
 but the compiler is within its rights to evaluate the operands in
 any order it wants.

For ia64, it would be good to evaluate the operands in parallel.
One could do the same with naked Transmeta hardware and other VLIW
processors.

Tera's machine has cheap hardware threads that could be used.
Compaq's 21364 or 21464 may allow this too.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Tom Leete

[CC trimmed]

Bill Wendling wrote:
> 
> Also sprach Tom Leete:
> }
> } You are correct that in C the rightmost argument is always
> } at the open end of the stack, and that varargs require that.
> } The opposite is called the Pascal convention.
> }
> Where in the standard does it say this? It's probably done most of the
> time in this fashion for convenience, but I don't believe it's in the
> standard.
> 

The standard is careful to avoid implementation/ABI issues.
It doesn't specify the use of a stack at all.

Nonetheless, there exists a C convention for stack based
archs, which is as I described. Pascal convention can
accomodate varargs by indirection, but that is OT.

My point was that stack order of values is independent of
the order of evaluation.

gcc is neither dumb nor psychic. It will not honor a
sequence point which exists only in the programmer's head.

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Jeff Epler

On Tue, Oct 17, 2000 at 07:42:31AM +, Peter Samuelson wrote:
> So, if you choose left-to-right, how do you implement varargs?  Keep in
> mind that prototypes are optional.  I bet anything your solution is
> slower and more complex than right-to-left, as all known compilers do.

Calling a varargs function with no prototype in scope is undefined
behavior.

For instance, when I designed a C calling convention in a class long
ago (for some mips-li[kt]e instruction set), I decided that the first
few arguments of normal functions would be passed in registers, but in
varargs functions all arguments would be passed on the stack.  This is a
perfectly legal under the C standard, even though calling printf without
a declaration visible would break.

Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Richard Henderson

>   [Matti Aarnio]
> > >   That depends mainly on question:  Does your stack grow up or down ?

Actually a combination -- which direction do arguments grow in relation
to stack growth direction, and do you have a stack push instruction.

> [Ben Pfaff]
> > No it doesn't.  It depends mainly on whether the ABI for the machine
> > says that arguments are pushed onto the stack in left-to-right or
> > right-to-left order.  You could do either on x86, for instance, and
> > it has nothing to do with whether the x86 stack grows up or down.

No, that's only true if your target supports stack pushes.

For instance, gcc on x86/linux will evaluate arguments right-to-left,
since it hopes to push arguments to the stack in that order.  But 
gcc on sparc will evaluate arguments mostly left-to-right, even though
the arguments are stored on the stack in the same layout as x86.

And actually it's more complicated than that, since I believe that
on most RISC we'll evaluate the arguments that wind up on the stack
first, left-to-right, then the arguments that wind up in registers, 
left-to-right.

On Tue, Oct 17, 2000 at 07:42:31AM +, Peter Samuelson wrote:
> So, if you choose left-to-right, how do you implement varargs?  Keep in
> mind that prototypes are optional.  I bet anything your solution is
> slower and more complex than right-to-left, as all known compilers do.

Speak ye little of "all known compilers".

It is possible to implement varargs however you like.  Up, down, or
sideways, every direction is used by some target somewhere.



r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Richard Henderson

On Tue, Oct 17, 2000 at 10:40:12AM +0100, Bernd Schmidt wrote:
> > I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
> > the assembler code.  The old code is 17 instructions long and the new code 
> > is 11 instructions.  As well as being shorter, simple timing test indicate 
> > that the new code is significantly quicker.
> 
> This is something the compiler ought to know about.

Ought to, but we mostly only rewrite addresses like that inside loops.
You might be able to convince CSE to massage that sort of thing, but I
imagine it would be ugly.

It would be cool though -- get everything into direct offset form as
much as possible might even make it easier to transform it all back
to autoinc if the target supports that sort of thing.


r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Raja R Harinath

Bill Wendling <[EMAIL PROTECTED]> writes:
> Also sprach Bernd Schmidt:
> } > Looking at the above code, I noticed that there are a lot of ++ 
> } > operations.  I rewrote the code as:
> } > 
> } >  setup_from[0] = setup_from[1] = eaddrs[0];
> } >  setup_from[2] = setup_from[3] = eaddrs[1];
> } >  setup_from[4] = setup_from[5] = eaddrs[2];
> } >  setup_from += 6;
> } > 
> } > I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486"
> } > to generate the assembler code.  The old code is 17 instructions
> } > long and the new code is 11 instructions.  As well as being
> } > shorter, simple timing test indicate that the new code is
> } > significantly quicker.
> } 
> } This is something the compiler ought to know about.
>  
> A compiler can only guess so much. Don't look to a compiler to fix poorly
> written code...

While I agree with this sentiment, I'll probably go with what Bernd
Schmidt says in this case.  He is, after all, the expert on that part
of GCC (CSE, loop opts, flow, etc.).

- Hari
-- 
Raja R Harinath -- [EMAIL PROTECTED]
"When all else fails, read the instructions."  -- Cahn's Axiom
"Our policy is, when in doubt, do the right thing."   -- Roy L Ash
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

On Tue, 17 Oct 2000, Horst von Brand wrote:

> Richard Guenther <[EMAIL PROTECTED]> said:
> > The following one is wrong, tho - should be rather
> > str[i] = dn[i]; i++;
> 
> > > diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c linux-2.4-fixe
> > d/drivers/isdn/sc/debug.c
> > > --- linux-2.4/drivers/isdn/sc/debug.c Thu Apr  2 01:21:04 1998
> > > +++ linux-2.4-fixed/drivers/isdn/sc/debug.c   Mon Oct 16 14:53:49 2000
> > > @@ -70,6 +70,6 @@
> > >   int i = 0;
> > >  
> > >   while(dn[i] != ',')
> > > - str[i] = dn[i++];
> > > + str[i] = dn[i], i++;
> > >   str[i] = 0x0;
> 
> What is wrong with plain and simple:
> 
>for(i = 0; dn[i] != ','; i++)
>   str[i] = dn[i];
>str[i] = '\0';

Nothing.  Feel free to submit a better patch.  I was just trying to correct
the code with the minimum set of modifications so as to avoid breaking
anything by accident.


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Tom Leete:
} 
} You are correct that in C the rightmost argument is always
} at the open end of the stack, and that varargs require that.
} The opposite is called the Pascal convention.
} 
Where in the standard does it say this? It's probably done most of the
time in this fashion for convenience, but I don't believe it's in the
standard.

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Bernd Schmidt:
} > Looking at the above code, I noticed that there are a lot of ++ 
} > operations.  I rewrote the code as:
} > 
} >  setup_from[0] = setup_from[1] = eaddrs[0];
} >  setup_from[2] = setup_from[3] = eaddrs[1];
} >  setup_from[4] = setup_from[5] = eaddrs[2];
} >  setup_from += 6;
} > 
} > I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
} > the assembler code.  The old code is 17 instructions long and the new code 
} > is 11 instructions.  As well as being shorter, simple timing test indicate 
} > that the new code is significantly quicker.
} 
} This is something the compiler ought to know about.
} 
A compiler can only guess so much. Don't look to a compiler to fix poorly
written code...

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Mark Montague:
} 
} dn[i], i++ evaluates to i, but str[i] = dn[i], i++ sets str[i] to
} dn[i] first, then increments i returning its previous value, which is
} discarded. Which was probably specified this way so
} 
} for(i=1,j=2; something; something else)
} 
} works as expected, rather than setting i and j to 2, and discarding
} the 1.
} 
Um...it sets i to 1 and j to 2.

} Just to encourage empiricism, I usually check stuff like that with
} 
} % cat > foo.c
} main(){
} int i;
} i = 1 , 2;
} printf("%d\n",i);
} }
} % gcc foo.c
} % ./a.out
} 1
} 
} or similar if I'm confused.
}
This isn't a good test of what's proper according to the standard. If you
are testing some undefined part of the standard, a compile may get it
"right" in one version but is perfectly within its rights to do something
else in the next version. The only way to solve these things is to look
at what the standard says and follow it. All else is hopes and wishes.
The above code is specified as working in the standard (evaluating the
assign and then evaluating the 2).

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Horst von Brand

Bernd Schmidt <[EMAIL PROTECTED]> said:
> On Tue, 17 Oct 2000, Richard Guenther wrote:
> > On Tue, 17 Oct 2000, Bernd Schmidt wrote:
> > > On Tue, 17 Oct 2000, Richard Guenther wrote:
> > > > The following one is wrong, tho - should be rather
> > > > str[i] = dn[i]; i++;
> > > 
> > > Nope.  (Well, at least you need to add extra braces.)  The comma is a
> > > sequence point.

> > Umm, I thought, dn[i], i++ evaluates to i and dn[i++] evaluates
> > to dn[i], so it should be either i++, dn[i-1] or the one I showed
> > above?

> According to operator precedence rules,
>   str[i] = dn[i], i++;
> is equivalent to
>   (str[i] = dn[i]), i++;

> Comma has the lowest precedence of all C operators.

If knowledgeable C users have doubts on the code's meaning, *rewrite
it*. This is just misunderstandings (and bugs) waiting for the unwary.
-- 
Dr. Horst H. von Brand   mailto:[EMAIL PROTECTED]
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria  +56 32 654239
Casilla 110-V, Valparaiso, ChileFax:  +56 32 797513
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Horst von Brand

Richard Guenther <[EMAIL PROTECTED]> said:
> On Mon, 16 Oct 2000, Bernd Schmidt wrote:
> The following one is wrong, tho - should be rather
> str[i] = dn[i]; i++;

> > diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c linux-2.4-fixe
> d/drivers/isdn/sc/debug.c
> > --- linux-2.4/drivers/isdn/sc/debug.c   Thu Apr  2 01:21:04 1998
> > +++ linux-2.4-fixed/drivers/isdn/sc/debug.c Mon Oct 16 14:53:49 2000
> > @@ -70,6 +70,6 @@
> > int i = 0;
> >  
> > while(dn[i] != ',')
> > -   str[i] = dn[i++];
> > +   str[i] = dn[i], i++;
> > str[i] = 0x0;

What is wrong with plain and simple:

   for(i = 0; dn[i] != ','; i++)
str[i] = dn[i];
   str[i] = '\0';

This one even has a chance of being understood... if you want fancy fooling
around, even this is more readable than the original:

  char *p = dn, *q = str;

  while((*q++ = *p++) != ',')
;
  *q = '\0';

Even so, all 3 _will_ break when a ','-less dn comes along...
-- 
Dr. Horst H. von Brand   mailto:[EMAIL PROTECTED]
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria  +56 32 654239
Casilla 110-V, Valparaiso, ChileFax:  +56 32 797513
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Tom Leete

Peter Samuelson wrote:
> 
>   [Matti Aarnio]
> > >   That depends mainly on question:  Does your stack grow up or down ?
> 
> [Ben Pfaff]
> > No it doesn't.  It depends mainly on whether the ABI for the machine
> > says that arguments are pushed onto the stack in left-to-right or
> > right-to-left order.  You could do either on x86, for instance, and
> > it has nothing to do with whether the x86 stack grows up or down.
> 
> So, if you choose left-to-right, how do you implement varargs?  Keep in
> mind that prototypes are optional.  I bet anything your solution is
> slower and more complex than right-to-left, as all known compilers do.
> 
> Peter

You are correct that in C the rightmost argument is always
at the open end of the stack, and that varargs require that.
The opposite is called the Pascal convention.

This thread has incorrectly identified the order of
expression evaluation with the physical stack layout of the
resulting values. The two properties are orthogonal. The
compiler is free to evaluate arguments in any order, and
knows which stack slot gets each result.

For instance, evaluation reordering might be done to
optimize cache coherency. It is conceivable that a SMP-aware
compiler would evaluate heavyweight arguments in parallel
(though currently Linux would not support that).

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Mark Montague

Richard Guenther <[EMAIL PROTECTED]> writes:

> On Tue, 17 Oct 2000, Bernd Schmidt wrote:
> 
> > On Tue, 17 Oct 2000, Richard Guenther wrote:
> > 
> > > On Mon, 16 Oct 2000, Bernd Schmidt wrote:
> > > 
> > > > I've been playing with some gcc patches to detect code with undefined
> > > > behaviour of the i = i++ variety.  The patch below fixes all places in
> > > > the kernel that I could find.  Note that in some cases, it wasn't
> > > > entirely clear what the code intended, so I had to guess.
> > > > 
> > > > I haven't tested this patch at all other than to make sure it compiles.
> > > 
> > > The following one is wrong, tho - should be rather
> > > str[i] = dn[i]; i++;
> > 
> > Nope.  (Well, at least you need to add extra braces.)  The comma is a
> > sequence point.
> 
> Umm, I thought, dn[i], i++ evaluates to i and dn[i++] evaluates
> to dn[i], so it should be either i++, dn[i-1] or the one I showed
> above?

dn[i], i++ evaluates to i, but str[i] = dn[i], i++ sets str[i] to
dn[i] first, then increments i returning its previous value, which is
discarded. Which was probably specified this way so

for(i=1,j=2; something; something else)

works as expected, rather than setting i and j to 2, and discarding
the 1.

Just to encourage empiricism, I usually check stuff like that with

% cat > foo.c
main(){
int i;
i = 1 , 2;
printf("%d\n",i);
}
% gcc foo.c
% ./a.out
1

or similar if I'm confused.

- M

-- 
Mark "Monty" Montague | [EMAIL PROTECTED]  | I don't do Windows(tm)
I'm dubious about any company whose assets can be destroyed by rm -rf
  http://www.gg.caltech.edu/~monty/monty.shtml>
 X-PGP-Fingerprint: E4 EA 6D B1 82 46 DB A1  B0 FF 60 B9 F9 5D 5C F7
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

On Tue, 17 Oct 2000, Richard Guenther wrote:
> On Tue, 17 Oct 2000, Bernd Schmidt wrote:
> > On Tue, 17 Oct 2000, Richard Guenther wrote:
> > > The following one is wrong, tho - should be rather
> > > str[i] = dn[i]; i++;
> > 
> > Nope.  (Well, at least you need to add extra braces.)  The comma is a
> > sequence point.
> 
> Umm, I thought, dn[i], i++ evaluates to i and dn[i++] evaluates
> to dn[i], so it should be either i++, dn[i-1] or the one I showed
> above?

According to operator precedence rules,
str[i] = dn[i], i++;
is equivalent to
(str[i] = dn[i]), i++;

Comma has the lowest precedence of all C operators.


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Richard Guenther

On Tue, 17 Oct 2000, Bernd Schmidt wrote:

> On Tue, 17 Oct 2000, Richard Guenther wrote:
> 
> > On Mon, 16 Oct 2000, Bernd Schmidt wrote:
> > 
> > > I've been playing with some gcc patches to detect code with undefined
> > > behaviour of the i = i++ variety.  The patch below fixes all places in
> > > the kernel that I could find.  Note that in some cases, it wasn't
> > > entirely clear what the code intended, so I had to guess.
> > > 
> > > I haven't tested this patch at all other than to make sure it compiles.
> > 
> > The following one is wrong, tho - should be rather
> > str[i] = dn[i]; i++;
> 
> Nope.  (Well, at least you need to add extra braces.)  The comma is a
> sequence point.

Umm, I thought, dn[i], i++ evaluates to i and dn[i++] evaluates
to dn[i], so it should be either i++, dn[i-1] or the one I showed
above?

Richard.

> > > diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c 
>linux-2.4-fixed/drivers/isdn/sc/debug.c
> > > --- linux-2.4/drivers/isdn/sc/debug.c Thu Apr  2 01:21:04 1998
> > > +++ linux-2.4-fixed/drivers/isdn/sc/debug.c   Mon Oct 16 14:53:49 2000
> > > @@ -70,6 +70,6 @@
> > >   int i = 0;
> > >  
> > >   while(dn[i] != ',')
> > > - str[i] = dn[i++];
> > > + str[i] = dn[i], i++;
> > >   str[i] = 0x0;
> > >  }
> 
> 
> Bernd
> 

--
Richard Guenther <[EMAIL PROTECTED]>
WWW: http://www.anatom.uni-tuebingen.de/~richi/
The GLAME Project: http://www.glame.de/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

> Looking at the above code, I noticed that there are a lot of ++ 
> operations.  I rewrote the code as:
> 
>  setup_from[0] = setup_from[1] = eaddrs[0];
>  setup_from[2] = setup_from[3] = eaddrs[1];
>  setup_from[4] = setup_from[5] = eaddrs[2];
>  setup_from += 6;
> 
> I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
> the assembler code.  The old code is 17 instructions long and the new code 
> is 11 instructions.  As well as being shorter, simple timing test indicate 
> that the new code is significantly quicker.

This is something the compiler ought to know about.


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

On Tue, 17 Oct 2000, Helge Hafting wrote:

> Bernd Schmidt wrote:
> > 
> > I've been playing with some gcc patches to detect code with undefined
> > behaviour of the i = i++ variety.  The patch below fixes all places in
> > the kernel that I could find.  Note that in some cases, it wasn't
> > entirely clear what the code intended, so I had to guess.
> 
> Please don't guess.  Look at the generated assembly, then make the
> unambigous code do whatever the old code did.  Chances are high it
> worked ok by luck, as misbehaving code tend to get noticed as
> it fails.

That does seem to be true for all parts except the tulip code.  I did
look at the assembly where I was in doubt.

Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

On Tue, 17 Oct 2000, Richard Guenther wrote:

> On Mon, 16 Oct 2000, Bernd Schmidt wrote:
> 
> > I've been playing with some gcc patches to detect code with undefined
> > behaviour of the i = i++ variety.  The patch below fixes all places in
> > the kernel that I could find.  Note that in some cases, it wasn't
> > entirely clear what the code intended, so I had to guess.
> > 
> > I haven't tested this patch at all other than to make sure it compiles.
> 
> The following one is wrong, tho - should be rather
> str[i] = dn[i]; i++;

Nope.  (Well, at least you need to add extra braces.)  The comma is a
sequence point.

> > diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c 
>linux-2.4-fixed/drivers/isdn/sc/debug.c
> > --- linux-2.4/drivers/isdn/sc/debug.c   Thu Apr  2 01:21:04 1998
> > +++ linux-2.4-fixed/drivers/isdn/sc/debug.c Mon Oct 16 14:53:49 2000
> > @@ -70,6 +70,6 @@
> > int i = 0;
> >  
> > while(dn[i] != ',')
> > -   str[i] = dn[i++];
> > +   str[i] = dn[i], i++;
> > str[i] = 0x0;
> >  }


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Richard Guenther

On Mon, 16 Oct 2000, Bernd Schmidt wrote:

> I've been playing with some gcc patches to detect code with undefined
> behaviour of the i = i++ variety.  The patch below fixes all places in
> the kernel that I could find.  Note that in some cases, it wasn't
> entirely clear what the code intended, so I had to guess.
> 
> I haven't tested this patch at all other than to make sure it compiles.

The following one is wrong, tho - should be rather
str[i] = dn[i]; i++;

> diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c 
>linux-2.4-fixed/drivers/isdn/sc/debug.c
> --- linux-2.4/drivers/isdn/sc/debug.c Thu Apr  2 01:21:04 1998
> +++ linux-2.4-fixed/drivers/isdn/sc/debug.c   Mon Oct 16 14:53:49 2000
> @@ -70,6 +70,6 @@
>   int i = 0;
>  
>   while(dn[i] != ',')
> - str[i] = dn[i++];
> + str[i] = dn[i], i++;
>   str[i] = 0x0;
>  }



--
Richard Guenther <[EMAIL PROTECTED]>
WWW: http://www.anatom.uni-tuebingen.de/~richi/
The GLAME Project: http://www.glame.de/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Helge Hafting

Bernd Schmidt wrote:
> 
> I've been playing with some gcc patches to detect code with undefined
> behaviour of the i = i++ variety.  The patch below fixes all places in
> the kernel that I could find.  Note that in some cases, it wasn't
> entirely clear what the code intended, so I had to guess.

Please don't guess.  Look at the generated assembly, then make the
unambigous code do whatever the old code did.  Chances are high it
worked ok by luck, as misbehaving code tend to get noticed as
it fails.

Helge Hafting
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Peter Samuelson


  [Matti Aarnio]
> >   That depends mainly on question:  Does your stack grow up or down ?

[Ben Pfaff]
> No it doesn't.  It depends mainly on whether the ABI for the machine
> says that arguments are pushed onto the stack in left-to-right or
> right-to-left order.  You could do either on x86, for instance, and
> it has nothing to do with whether the x86 stack grows up or down.

So, if you choose left-to-right, how do you implement varargs?  Keep in
mind that prototypes are optional.  I bet anything your solution is
slower and more complex than right-to-left, as all known compilers do.

Peter
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Tom Leete

Peter Samuelson wrote:
 
   [Matti Aarnio]
 That depends mainly on question:  Does your stack grow up or down ?
 
 [Ben Pfaff]
  No it doesn't.  It depends mainly on whether the ABI for the machine
  says that arguments are pushed onto the stack in left-to-right or
  right-to-left order.  You could do either on x86, for instance, and
  it has nothing to do with whether the x86 stack grows up or down.
 
 So, if you choose left-to-right, how do you implement varargs?  Keep in
 mind that prototypes are optional.  I bet anything your solution is
 slower and more complex than right-to-left, as all known compilers do.
 
 Peter

You are correct that in C the rightmost argument is always
at the open end of the stack, and that varargs require that.
The opposite is called the Pascal convention.

This thread has incorrectly identified the order of
expression evaluation with the physical stack layout of the
resulting values. The two properties are orthogonal. The
compiler is free to evaluate arguments in any order, and
knows which stack slot gets each result.

For instance, evaluation reordering might be done to
optimize cache coherency. It is conceivable that a SMP-aware
compiler would evaluate heavyweight arguments in parallel
(though currently Linux would not support that).

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Horst von Brand

Richard Guenther [EMAIL PROTECTED] said:
 On Mon, 16 Oct 2000, Bernd Schmidt wrote:
 The following one is wrong, tho - should be rather
 str[i] = dn[i]; i++;

  diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c linux-2.4-fixe
 d/drivers/isdn/sc/debug.c
  --- linux-2.4/drivers/isdn/sc/debug.c   Thu Apr  2 01:21:04 1998
  +++ linux-2.4-fixed/drivers/isdn/sc/debug.c Mon Oct 16 14:53:49 2000
  @@ -70,6 +70,6 @@
  int i = 0;
   
  while(dn[i] != ',')
  -   str[i] = dn[i++];
  +   str[i] = dn[i], i++;
  str[i] = 0x0;

What is wrong with plain and simple:

   for(i = 0; dn[i] != ','; i++)
str[i] = dn[i];
   str[i] = '\0';

This one even has a chance of being understood... if you want fancy fooling
around, even this is more readable than the original:

  char *p = dn, *q = str;

  while((*q++ = *p++) != ',')
;
  *q = '\0';

Even so, all 3 _will_ break when a ','-less dn comes along...
-- 
Dr. Horst H. von Brand   mailto:[EMAIL PROTECTED]
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria  +56 32 654239
Casilla 110-V, Valparaiso, ChileFax:  +56 32 797513
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Horst von Brand

Bernd Schmidt [EMAIL PROTECTED] said:
 On Tue, 17 Oct 2000, Richard Guenther wrote:
  On Tue, 17 Oct 2000, Bernd Schmidt wrote:
   On Tue, 17 Oct 2000, Richard Guenther wrote:
The following one is wrong, tho - should be rather
str[i] = dn[i]; i++;
   
   Nope.  (Well, at least you need to add extra braces.)  The comma is a
   sequence point.

  Umm, I thought, dn[i], i++ evaluates to i and dn[i++] evaluates
  to dn[i], so it should be either i++, dn[i-1] or the one I showed
  above?

 According to operator precedence rules,
   str[i] = dn[i], i++;
 is equivalent to
   (str[i] = dn[i]), i++;

 Comma has the lowest precedence of all C operators.

If knowledgeable C users have doubts on the code's meaning, *rewrite
it*. This is just misunderstandings (and bugs) waiting for the unwary.
-- 
Dr. Horst H. von Brand   mailto:[EMAIL PROTECTED]
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria  +56 32 654239
Casilla 110-V, Valparaiso, ChileFax:  +56 32 797513
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Mark Montague:
} 
} dn[i], i++ evaluates to i, but str[i] = dn[i], i++ sets str[i] to
} dn[i] first, then increments i returning its previous value, which is
} discarded. Which was probably specified this way so
} 
} for(i=1,j=2; something; something else)
} 
} works as expected, rather than setting i and j to 2, and discarding
} the 1.
} 
Um...it sets i to 1 and j to 2.

} Just to encourage empiricism, I usually check stuff like that with
} 
} % cat  foo.c
} main(){
} int i;
} i = 1 , 2;
} printf("%d\n",i);
} }
} % gcc foo.c
} % ./a.out
} 1
} 
} or similar if I'm confused.
}
This isn't a good test of what's proper according to the standard. If you
are testing some undefined part of the standard, a compile may get it
"right" in one version but is perfectly within its rights to do something
else in the next version. The only way to solve these things is to look
at what the standard says and follow it. All else is hopes and wishes.
The above code is specified as working in the standard (evaluating the
assign and then evaluating the 2).

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Bernd Schmidt:
}  Looking at the above code, I noticed that there are a lot of ++ 
}  operations.  I rewrote the code as:
}  
}   setup_from[0] = setup_from[1] = eaddrs[0];
}   setup_from[2] = setup_from[3] = eaddrs[1];
}   setup_from[4] = setup_from[5] = eaddrs[2];
}   setup_from += 6;
}  
}  I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
}  the assembler code.  The old code is 17 instructions long and the new code 
}  is 11 instructions.  As well as being shorter, simple timing test indicate 
}  that the new code is significantly quicker.
} 
} This is something the compiler ought to know about.
} 
A compiler can only guess so much. Don't look to a compiler to fix poorly
written code...

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bill Wendling

Also sprach Tom Leete:
} 
} You are correct that in C the rightmost argument is always
} at the open end of the stack, and that varargs require that.
} The opposite is called the Pascal convention.
} 
Where in the standard does it say this? It's probably done most of the
time in this fashion for convenience, but I don't believe it's in the
standard.

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Bernd Schmidt

On Tue, 17 Oct 2000, Horst von Brand wrote:

 Richard Guenther [EMAIL PROTECTED] said:
  The following one is wrong, tho - should be rather
  str[i] = dn[i]; i++;
 
   diff -x log.build -x .* -dru linux-2.4/drivers/isdn/sc/debug.c linux-2.4-fixe
  d/drivers/isdn/sc/debug.c
   --- linux-2.4/drivers/isdn/sc/debug.c Thu Apr  2 01:21:04 1998
   +++ linux-2.4-fixed/drivers/isdn/sc/debug.c   Mon Oct 16 14:53:49 2000
   @@ -70,6 +70,6 @@
 int i = 0;

 while(dn[i] != ',')
   - str[i] = dn[i++];
   + str[i] = dn[i], i++;
 str[i] = 0x0;
 
 What is wrong with plain and simple:
 
for(i = 0; dn[i] != ','; i++)
   str[i] = dn[i];
str[i] = '\0';

Nothing.  Feel free to submit a better patch.  I was just trying to correct
the code with the minimum set of modifications so as to avoid breaking
anything by accident.


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Raja R Harinath

Bill Wendling [EMAIL PROTECTED] writes:
 Also sprach Bernd Schmidt:
 }  Looking at the above code, I noticed that there are a lot of ++ 
 }  operations.  I rewrote the code as:
 }  
 }   setup_from[0] = setup_from[1] = eaddrs[0];
 }   setup_from[2] = setup_from[3] = eaddrs[1];
 }   setup_from[4] = setup_from[5] = eaddrs[2];
 }   setup_from += 6;
 }  
 }  I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486"
 }  to generate the assembler code.  The old code is 17 instructions
 }  long and the new code is 11 instructions.  As well as being
 }  shorter, simple timing test indicate that the new code is
 }  significantly quicker.
 } 
 } This is something the compiler ought to know about.
  
 A compiler can only guess so much. Don't look to a compiler to fix poorly
 written code...

While I agree with this sentiment, I'll probably go with what Bernd
Schmidt says in this case.  He is, after all, the expert on that part
of GCC (CSE, loop opts, flow, etc.).

- Hari
-- 
Raja R Harinath -- [EMAIL PROTECTED]
"When all else fails, read the instructions."  -- Cahn's Axiom
"Our policy is, when in doubt, do the right thing."   -- Roy L Ash
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Richard Henderson

On Tue, Oct 17, 2000 at 10:40:12AM +0100, Bernd Schmidt wrote:
  I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
  the assembler code.  The old code is 17 instructions long and the new code 
  is 11 instructions.  As well as being shorter, simple timing test indicate 
  that the new code is significantly quicker.
 
 This is something the compiler ought to know about.

Ought to, but we mostly only rewrite addresses like that inside loops.
You might be able to convince CSE to massage that sort of thing, but I
imagine it would be ugly.

It would be cool though -- get everything into direct offset form as
much as possible might even make it easier to transform it all back
to autoinc if the target supports that sort of thing.


r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Jeff Epler

On Tue, Oct 17, 2000 at 07:42:31AM +, Peter Samuelson wrote:
 So, if you choose left-to-right, how do you implement varargs?  Keep in
 mind that prototypes are optional.  I bet anything your solution is
 slower and more complex than right-to-left, as all known compilers do.

Calling a varargs function with no prototype in scope is undefined
behavior.

For instance, when I designed a C calling convention in a class long
ago (for some mips-li[kt]e instruction set), I decided that the first
few arguments of normal functions would be passed in registers, but in
varargs functions all arguments would be passed on the stack.  This is a
perfectly legal under the C standard, even though calling printf without
a declaration visible would break.

Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-17 Thread Tom Leete

[CC trimmed]

Bill Wendling wrote:
 
 Also sprach Tom Leete:
 }
 } You are correct that in C the rightmost argument is always
 } at the open end of the stack, and that varargs require that.
 } The opposite is called the Pascal convention.
 }
 Where in the standard does it say this? It's probably done most of the
 time in this fashion for convenience, but I don't believe it's in the
 standard.
 

The standard is careful to avoid implementation/ABI issues.
It doesn't specify the use of a stack at all.

Nonetheless, there exists a C convention for stack based
archs, which is as I described. Pascal convention can
accomodate varargs by indirection, but that is OT.

My point was that stack order of values is independent of
the order of evaluation.

gcc is neither dumb nor psychic. It will not honor a
sequence point which exists only in the programmer's head.

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Matti Aarnio <[EMAIL PROTECTED]> writes:

> On Mon, Oct 16, 2000 at 04:55:08PM -0400, Ben Pfaff wrote:
> > Yes.  In practice the usual question is whether the compiler will
> > evaluate the operands from left to right or from right to left,
> > but the compiler is within its rights to evaluate the operands in
> > any order it wants.
> 
>   That depends mainly on question:  Does your stack grow up or down ?

No it doesn't.  It depends mainly on whether the ABI for the
machine says that arguments are pushed onto the stack in
left-to-right or right-to-left order.  You could do either on
x86, for instance, and it has nothing to do with whether the x86
stack grows up or down.

> > > Does this imply that even the evaluation of a function pointer
> > > is itself undefined in terms of ordering?
> > 
> > Yes.
>
>For things like lone pointer referral with pre/post inc/dec:
>   *p++
>it is well defined, but for things like:
>   *p++ + *p
>it is not.  (Will the second *p be evaluated before or after *p++
>post-increment ?)

Non sequitur: I see no function pointers being evaluated here.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Matti Aarnio

On Mon, Oct 16, 2000 at 04:55:08PM -0400, Ben Pfaff wrote:
> > >   $ The order of evaluation of the function designator, the
> > >   $ arguments, and subexpressions within the arguments is
> > >   $ unspecified, ...
> > 
> > I sit surprised and corrected.  With every version of every C compiler on
> > every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
> > Manx C to E1 using Kai C++) the behavior has been the same for parameter
> > lists as for the comma operator in this respect.
> 
> Yes.  In practice the usual question is whether the compiler will
> evaluate the operands from left to right or from right to left,
> but the compiler is within its rights to evaluate the operands in
> any order it wants.

  That depends mainly on question:  Does your stack grow up or down ?

  Machines where stack grows up are a bit rare, but they do exist.
  The CISC archetype, IBM S/360/370/390 has simple instruction to
  add a small positive offset value into pointer register, but not
  to substract it (nor have negative offsets). Thus most stackfull
  languages there have the stack growing up.  Doing downwards-growing
  stack setup requires 1 extra register use, and one extra instruction
  per frame entry, plus considerably more for parameter push.
  (But it was a great beast to run Fortran-IV which didn't need stack,
   just subfunction invocation record -- at a static location unique
   for each subfunction --> no recursion supported.)

> > Does this imply that even the evaluation of a function pointer
> > is itself undefined in terms of ordering?
> 
> Yes.


   For things like lone pointer referral with pre/post inc/dec:
*p++
   it is well defined, but for things like:
*p++ + *p
   it is not.  (Will the second *p be evaluated before or after *p++
   post-increment ?)


/Matti Aarnio
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Alexander Viro



On Mon, 16 Oct 2000, Mike Castle wrote:

> On Mon, Oct 16, 2000 at 04:47:09PM -0400, Alexander Viro wrote:
> > tmp = *p++;
> > *q = f(tmp, *p++);
> > return p;
> > 
> > is equivalent to more idiomatic
> > 
> > *q = f(p[0], p[1]);
> > return p+2;
> 
> 
> Which gets better assembler out of various versions of gcc?

On which platform? If it would be VAX - sure, autoincrement rocks,
but for something like x86 I would expect the second form to do better.
Besides, it's more readable and is harder to fsck up when you are
modifying the code.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Mike Castle

On Mon, Oct 16, 2000 at 04:47:09PM -0400, Alexander Viro wrote:
> tmp = *p++;
> *q = f(tmp, *p++);
> return p;
> 
> is equivalent to more idiomatic
> 
> *q = f(p[0], p[1]);
> return p+2;


Which gets better assembler out of various versions of gcc?

mrc
-- 
   Mike Castle   Life is like a clock:  You can work constantly
  [EMAIL PROTECTED]  and be right all the time, or not work at all
www.netcom.com/~dalgoda/ and be right at least twice a day.  -- mrc
We are all of us living in the shadow of Manhattan.  -- Watchmen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



RE: Patch to remove undefined C code

2000-10-16 Thread Jonathan George

>-Original Message-
>From: Alexander Viro [mailto:[EMAIL PROTECTED]]
[snip]
>
>No arguments here, but proposed fixes were remarkably ugly. Example:
>
>tmp = *p++;
>*q = f(tmp, *p++);
>return p;
>
>is equivalent to more idiomatic
>
>*q = f(p[0], p[1]);
>return p+2;
>
>And example with copying the string up to the comma... Yuck. Legal C !=
>decent C.

Strongly agree.  If the changes were an elegant solution to the ambiguities
I never would have cared.  In fact the whole reason I bothered to read the
patch in the first place was that I feel strongly that something like that
_was_ needed, but the solutions really seemed inferior to the original code
in terms of elegance of expression. (Well some of the original code was
pretty ugly too :-)

--Jonathan--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Jonathan George <[EMAIL PROTECTED]> writes:

> > $ The order of evaluation of the function designator, the
> > $ arguments, and subexpressions within the arguments is
> > $ unspecified, ...
> >--
> 
> I sit surprised and corrected.  With every version of every C compiler on
> every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
> Manx C to E1 using Kai C++) the behavior has been the same for parameter
> lists as for the comma operator in this respect.

Yes.  In practice the usual question is whether the compiler will
evaluate the operands from left to right or from right to left,
but the compiler is within its rights to evaluate the operands in
any order it wants.

> Does this imply that even the evaluation of a function pointer
> is itself undefined in terms of ordering?

Yes.

> >"Premature optimization is the root of all evil."
> >--D. E. Knuth, "Structured Programming with go to Statements"
> 
> Absolutely true.

Good sigmonster, have $TREAT.

> I would be really interested to see how well the kernel compiler does at
> optimizing out all of these new tmp variables in terms of the resulting
> register utilization and performance.  At least an assembler code size
> comparison is in order for each of these individual cases.

I agree with you and Al Viro that the proposed fixes were less
than desirable.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



RE: Patch to remove undefined C code

2000-10-16 Thread Jonathan George

-Original Message-
From: Ben Pfaff [mailto:[EMAIL PROTECTED]]
>Jonathan George <[EMAIL PROTECTED]> writes:
>
>> This patch has many bogus corrections where new variables were created,
but
>> the order of evaluation is already unambiguous.
>> 
>> For example each comma separated clause in an expression is guaranteed to
be
>> completely evaluated before the next comma separated clause Including
>> Assignments.
>
>No, that is not true in general.  When the comma in question is
>the comma operator, it is true.  But when the comma separates
>arguments to a function, it is not: the order of evaluation of
>function arguments is implementation dependent.  See C89 section
>6.3.2.2 "Function calls":
>
>   $ The order of evaluation of the function designator, the
>   $ arguments, and subexpressions within the arguments is
>   $ unspecified, ...
>--

I sit surprised and corrected.  With every version of every C compiler on
every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
Manx C to E1 using Kai C++) the behavior has been the same for parameter
lists as for the comma operator in this respect.  Does this imply that even
the evaluation of a function pointer is itself undefined in terms of
ordering?

>"Premature optimization is the root of all evil."
>--D. E. Knuth, "Structured Programming with go to Statements"

Absolutely true.

However, optimization of well tested, and algorithmically validated kernel
code in a time critical section does not constitute premature optimization
as I'm sure Donald would agree.

I would be really interested to see how well the kernel compiler does at
optimizing out all of these new tmp variables in terms of the resulting
register utilization and performance.  At least an assembler code size
comparison is in order for each of these individual cases.

That is my only real concern (other than all of that syntactically ambiguous
code I have written over the last 15 years ;-) is that critical performance
not be compromised in the kernel.

--Jonathan--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Alexander Viro



On 16 Oct 2000, Ben Pfaff wrote:

> Jonathan George <[EMAIL PROTECTED]> writes:
> 
> > This patch has many bogus corrections where new variables were created, but
> > the order of evaluation is already unambiguous.
> > 
> > For example each comma separated clause in an expression is guaranteed to be
> > completely evaluated before the next comma separated clause Including
> > Assignments.
> 
> No, that is not true in general.  When the comma in question is
> the comma operator, it is true.  But when the comma separates
> arguments to a function, it is not: the order of evaluation of
> function arguments is implementation dependent.  See C89 section
> 6.3.2.2 "Function calls":
[snip]

No arguments here, but proposed fixes were remarkably ugly. Example:

tmp = *p++;
*q = f(tmp, *p++);
return p;

is equivalent to more idiomatic

*q = f(p[0], p[1]);
return p+2;

And example with copying the string up to the comma... Yuck. Legal C !=
decent C.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Jonathan George <[EMAIL PROTECTED]> writes:

> This patch has many bogus corrections where new variables were created, but
> the order of evaluation is already unambiguous.
> 
> For example each comma separated clause in an expression is guaranteed to be
> completely evaluated before the next comma separated clause Including
> Assignments.

No, that is not true in general.  When the comma in question is
the comma operator, it is true.  But when the comma separates
arguments to a function, it is not: the order of evaluation of
function arguments is implementation dependent.  See C89 section
6.3.2.2 "Function calls":

$ The order of evaluation of the function designator, the
$ arguments, and subexpressions within the arguments is
$ unspecified, ...
-- 
"Premature optimization is the root of all evil."
--D. E. Knuth, "Structured Programming with go to Statements"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Bill Wendling

Also sprach Abramo Bagnara:
} 
} Isn't this more efficient?
}   n = (x>>32) | (x<<32); 
}   n = ((n & 0xLL)<<16) | (n & 0xLL)>>16; 
}   n = ((n & 0x00ff00ff00ff00ffLL)<<8) | (n & 0xff00ff00ff00ff00LL)>>8; 
} 
} 6 shift
} 4 and
} 3 or
} 
Plus 3 assigns...but they may get optimized out. :)

} instead of
} 
} 8 shift
} 8 and
} 7 or
} 

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread David Relson

At 01:21 PM 10/16/00, Jeff Garzik wrote:
>Bernd Schmidt wrote:
> > diff -x log.build -x .* -dru linux-2.4/drivers/net/tulip/tulip_core.c 
> linux-2.4-fixed/drivers/net/tulip/tulip_core.c
> > --- linux-2.4/drivers/net/tulip/tulip_core.cMon Oct 16 13:51:23 2000
> > +++ linux-2.4-fixed/drivers/net/tulip/tulip_core.c  Mon Oct 16 
> 15:40:12 2000
>[...]
> > @@ -944,9 +946,9 @@
> >
> > /* Fill the final entry with our physical address. */
> > eaddrs = (u16 *)dev->dev_addr;
> > -   *setup_frm++ = *setup_frm++ = eaddrs[0];
> > -   *setup_frm++ = *setup_frm++ = eaddrs[1];
> > -   *setup_frm++ = *setup_frm++ = eaddrs[2];
> > +   *setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
> > +   *setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
> > +   *setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
> >
> > spin_lock_irqsave(>lock, flags);
> >

Looking at the above code, I noticed that there are a lot of ++ 
operations.  I rewrote the code as:

 setup_from[0] = setup_from[1] = eaddrs[0];
 setup_from[2] = setup_from[3] = eaddrs[1];
 setup_from[4] = setup_from[5] = eaddrs[2];
 setup_from += 6;

I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
the assembler code.  The old code is 17 instructions long and the new code 
is 11 instructions.  As well as being shorter, simple timing test indicate 
that the new code is significantly quicker.

David


David Relson   Osage Software Systems, Inc.
[EMAIL PROTECTED]   514 W. Keech Ave.
www.osagesoftware.com  Ann Arbor, MI 48103
voice: 734.821.8800fax: 734.821.8800

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Abramo Bagnara

Bernd Schmidt wrote:
> 
> +
> +#define ___swab64(x) \
> +({ \
> +   __u64 __x = (x); \
> +   ((__u64)( \
> +   (__u64)(((__u64)(__x) & (__u64)0x00ffULL) << 56) | \
> +   (__u64)(((__u64)(__x) & (__u64)0xff00ULL) << 40) | \
> +   (__u64)(((__u64)(__x) & (__u64)0x00ffULL) << 24) | \
> +   (__u64)(((__u64)(__x) & (__u64)0xff00ULL) <<  8) | \
> +   (__u64)(((__u64)(__x) & (__u64)0x00ffULL) >>  8) | \
> +   (__u64)(((__u64)(__x) & (__u64)0xff00ULL) >> 24) | \
> +   (__u64)(((__u64)(__x) & (__u64)0x00ffULL) >> 40) | \
> +   (__u64)(((__u64)(__x) & (__u64)0xff00ULL) >> 56) )); \
> +})


Isn't this more efficient?
  n = (x>>32) | (x<<32); 
  n = ((n & 0xLL)<<16) | (n & 0xLL)>>16; 
  n = ((n & 0x00ff00ff00ff00ffLL)<<8) | (n & 0xff00ff00ff00ff00LL)>>8; 

6 shift
4 and
3 or

instead of

8 shift
8 and
7 or

-- 
Abramo Bagnara   mailto:[EMAIL PROTECTED]

Opera Unica
Via Emilia Interna, 140  Phone: +39.0546.656023
48014 Castel Bolognese (RA) - Italy  Fax:   +39.0546.656023

ALSA project ishttp://www.alsa-project.org
sponsored by SuSE Linuxhttp://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Bernd Schmidt

On Mon, 16 Oct 2000, Jeff Garzik wrote:

> > --- linux-2.4/drivers/scsi/aha152x.cMon Oct 16 13:51:24 2000
> > +++ linux-2.4-fixed/drivers/scsi/aha152x.c  Mon Oct 16 14:51:29 2000
> > @@ -1280,7 +1280,8 @@
> > scsi_unregister(shpnt);
> > registered_count--;
> > release_region(shpnt->io_port, IO_RANGE);
> > -   aha152x_host[shpnt->irq - IRQ_MIN] = shpnt = 0;
> > +   aha152x_host[shpnt->irq - IRQ_MIN] = 0;
> > +   shpnt = 0;
> > continue;
> > }
> 
> Why this change?

aha152x_host[shpnt->irq - IRQ_MIN] = shpnt = 0;
  reference   set


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Jeff Garzik

Bernd Schmidt wrote:
> diff -x log.build -x .* -dru linux-2.4/drivers/net/tulip/tulip_core.c 
>linux-2.4-fixed/drivers/net/tulip/tulip_core.c
> --- linux-2.4/drivers/net/tulip/tulip_core.cMon Oct 16 13:51:23 2000
> +++ linux-2.4-fixed/drivers/net/tulip/tulip_core.c  Mon Oct 16 15:40:12 2000
[...]
> @@ -944,9 +946,9 @@
> 
> /* Fill the final entry with our physical address. */
> eaddrs = (u16 *)dev->dev_addr;
> -   *setup_frm++ = *setup_frm++ = eaddrs[0];
> -   *setup_frm++ = *setup_frm++ = eaddrs[1];
> -   *setup_frm++ = *setup_frm++ = eaddrs[2];
> +   *setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
> +   *setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
> +   *setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
> 
> spin_lock_irqsave(>lock, flags);
> 

Linus/Bernd, please drop this patch to drivers/net/tulip/tulip_core.c, I
have a much better change from prumpf for this already in my CVS...


> --- linux-2.4/drivers/scsi/aha152x.cMon Oct 16 13:51:24 2000
> +++ linux-2.4-fixed/drivers/scsi/aha152x.c  Mon Oct 16 14:51:29 2000
> @@ -1280,7 +1280,8 @@
> scsi_unregister(shpnt);
> registered_count--;
> release_region(shpnt->io_port, IO_RANGE);
> -   aha152x_host[shpnt->irq - IRQ_MIN] = shpnt = 0;
> +   aha152x_host[shpnt->irq - IRQ_MIN] = 0;
> +   shpnt = 0;
> continue;
> }

Why this change?

Other than these two things, the changes look ok.

-- 
Jeff Garzik| The difference between laziness and
Building 1024  | prioritization is the end result.
MandrakeSoft   |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Jeff Garzik

Bernd Schmidt wrote:
 diff -x log.build -x .* -dru linux-2.4/drivers/net/tulip/tulip_core.c 
linux-2.4-fixed/drivers/net/tulip/tulip_core.c
 --- linux-2.4/drivers/net/tulip/tulip_core.cMon Oct 16 13:51:23 2000
 +++ linux-2.4-fixed/drivers/net/tulip/tulip_core.c  Mon Oct 16 15:40:12 2000
[...]
 @@ -944,9 +946,9 @@
 
 /* Fill the final entry with our physical address. */
 eaddrs = (u16 *)dev-dev_addr;
 -   *setup_frm++ = *setup_frm++ = eaddrs[0];
 -   *setup_frm++ = *setup_frm++ = eaddrs[1];
 -   *setup_frm++ = *setup_frm++ = eaddrs[2];
 +   *setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
 +   *setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
 +   *setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
 
 spin_lock_irqsave(tp-lock, flags);
 

Linus/Bernd, please drop this patch to drivers/net/tulip/tulip_core.c, I
have a much better change from prumpf for this already in my CVS...


 --- linux-2.4/drivers/scsi/aha152x.cMon Oct 16 13:51:24 2000
 +++ linux-2.4-fixed/drivers/scsi/aha152x.c  Mon Oct 16 14:51:29 2000
 @@ -1280,7 +1280,8 @@
 scsi_unregister(shpnt);
 registered_count--;
 release_region(shpnt-io_port, IO_RANGE);
 -   aha152x_host[shpnt-irq - IRQ_MIN] = shpnt = 0;
 +   aha152x_host[shpnt-irq - IRQ_MIN] = 0;
 +   shpnt = 0;
 continue;
 }

Why this change?

Other than these two things, the changes look ok.

-- 
Jeff Garzik| The difference between laziness and
Building 1024  | prioritization is the end result.
MandrakeSoft   |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Bernd Schmidt

On Mon, 16 Oct 2000, Jeff Garzik wrote:

  --- linux-2.4/drivers/scsi/aha152x.cMon Oct 16 13:51:24 2000
  +++ linux-2.4-fixed/drivers/scsi/aha152x.c  Mon Oct 16 14:51:29 2000
  @@ -1280,7 +1280,8 @@
  scsi_unregister(shpnt);
  registered_count--;
  release_region(shpnt-io_port, IO_RANGE);
  -   aha152x_host[shpnt-irq - IRQ_MIN] = shpnt = 0;
  +   aha152x_host[shpnt-irq - IRQ_MIN] = 0;
  +   shpnt = 0;
  continue;
  }
 
 Why this change?

aha152x_host[shpnt-irq - IRQ_MIN] = shpnt = 0;
  reference   set


Bernd

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Abramo Bagnara

Bernd Schmidt wrote:
 
 +
 +#define ___swab64(x) \
 +({ \
 +   __u64 __x = (x); \
 +   ((__u64)( \
 +   (__u64)(((__u64)(__x)  (__u64)0x00ffULL)  56) | \
 +   (__u64)(((__u64)(__x)  (__u64)0xff00ULL)  40) | \
 +   (__u64)(((__u64)(__x)  (__u64)0x00ffULL)  24) | \
 +   (__u64)(((__u64)(__x)  (__u64)0xff00ULL)   8) | \
 +   (__u64)(((__u64)(__x)  (__u64)0x00ffULL)   8) | \
 +   (__u64)(((__u64)(__x)  (__u64)0xff00ULL)  24) | \
 +   (__u64)(((__u64)(__x)  (__u64)0x00ffULL)  40) | \
 +   (__u64)(((__u64)(__x)  (__u64)0xff00ULL)  56) )); \
 +})


Isn't this more efficient?
  n = (x32) | (x32); 
  n = ((n  0xLL)16) | (n  0xLL)16; 
  n = ((n  0x00ff00ff00ff00ffLL)8) | (n  0xff00ff00ff00ff00LL)8; 

6 shift
4 and
3 or

instead of

8 shift
8 and
7 or

-- 
Abramo Bagnara   mailto:[EMAIL PROTECTED]

Opera Unica
Via Emilia Interna, 140  Phone: +39.0546.656023
48014 Castel Bolognese (RA) - Italy  Fax:   +39.0546.656023

ALSA project ishttp://www.alsa-project.org
sponsored by SuSE Linuxhttp://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread David Relson

At 01:21 PM 10/16/00, Jeff Garzik wrote:
Bernd Schmidt wrote:
  diff -x log.build -x .* -dru linux-2.4/drivers/net/tulip/tulip_core.c 
 linux-2.4-fixed/drivers/net/tulip/tulip_core.c
  --- linux-2.4/drivers/net/tulip/tulip_core.cMon Oct 16 13:51:23 2000
  +++ linux-2.4-fixed/drivers/net/tulip/tulip_core.c  Mon Oct 16 
 15:40:12 2000
[...]
  @@ -944,9 +946,9 @@
 
  /* Fill the final entry with our physical address. */
  eaddrs = (u16 *)dev-dev_addr;
  -   *setup_frm++ = *setup_frm++ = eaddrs[0];
  -   *setup_frm++ = *setup_frm++ = eaddrs[1];
  -   *setup_frm++ = *setup_frm++ = eaddrs[2];
  +   *setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
  +   *setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
  +   *setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
 
  spin_lock_irqsave(tp-lock, flags);
 

Looking at the above code, I noticed that there are a lot of ++ 
operations.  I rewrote the code as:

 setup_from[0] = setup_from[1] = eaddrs[0];
 setup_from[2] = setup_from[3] = eaddrs[1];
 setup_from[4] = setup_from[5] = eaddrs[2];
 setup_from += 6;

I compiled using "gcc -S -Wall -O2 -fomit-frame-pointer -m486" to generate 
the assembler code.  The old code is 17 instructions long and the new code 
is 11 instructions.  As well as being shorter, simple timing test indicate 
that the new code is significantly quicker.

David


David Relson   Osage Software Systems, Inc.
[EMAIL PROTECTED]   514 W. Keech Ave.
www.osagesoftware.com  Ann Arbor, MI 48103
voice: 734.821.8800fax: 734.821.8800

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Bill Wendling

Also sprach Abramo Bagnara:
} 
} Isn't this more efficient?
}   n = (x32) | (x32); 
}   n = ((n  0xLL)16) | (n  0xLL)16; 
}   n = ((n  0x00ff00ff00ff00ffLL)8) | (n  0xff00ff00ff00ff00LL)8; 
} 
} 6 shift
} 4 and
} 3 or
} 
Plus 3 assigns...but they may get optimized out. :)

} instead of
} 
} 8 shift
} 8 and
} 7 or
} 

-- 
|| Bill Wendling[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Jonathan George [EMAIL PROTECTED] writes:

 This patch has many bogus corrections where new variables were created, but
 the order of evaluation is already unambiguous.
 
 For example each comma separated clause in an expression is guaranteed to be
 completely evaluated before the next comma separated clause Including
 Assignments.

No, that is not true in general.  When the comma in question is
the comma operator, it is true.  But when the comma separates
arguments to a function, it is not: the order of evaluation of
function arguments is implementation dependent.  See C89 section
6.3.2.2 "Function calls":

$ The order of evaluation of the function designator, the
$ arguments, and subexpressions within the arguments is
$ unspecified, ...
-- 
"Premature optimization is the root of all evil."
--D. E. Knuth, "Structured Programming with go to Statements"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Alexander Viro



On 16 Oct 2000, Ben Pfaff wrote:

 Jonathan George [EMAIL PROTECTED] writes:
 
  This patch has many bogus corrections where new variables were created, but
  the order of evaluation is already unambiguous.
  
  For example each comma separated clause in an expression is guaranteed to be
  completely evaluated before the next comma separated clause Including
  Assignments.
 
 No, that is not true in general.  When the comma in question is
 the comma operator, it is true.  But when the comma separates
 arguments to a function, it is not: the order of evaluation of
 function arguments is implementation dependent.  See C89 section
 6.3.2.2 "Function calls":
[snip]

No arguments here, but proposed fixes were remarkably ugly. Example:

tmp = *p++;
*q = f(tmp, *p++);
return p;

is equivalent to more idiomatic

*q = f(p[0], p[1]);
return p+2;

And example with copying the string up to the comma... Yuck. Legal C !=
decent C.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



RE: Patch to remove undefined C code

2000-10-16 Thread Jonathan George

-Original Message-
From: Ben Pfaff [mailto:[EMAIL PROTECTED]]
Jonathan George [EMAIL PROTECTED] writes:

 This patch has many bogus corrections where new variables were created,
but
 the order of evaluation is already unambiguous.
 
 For example each comma separated clause in an expression is guaranteed to
be
 completely evaluated before the next comma separated clause Including
 Assignments.

No, that is not true in general.  When the comma in question is
the comma operator, it is true.  But when the comma separates
arguments to a function, it is not: the order of evaluation of
function arguments is implementation dependent.  See C89 section
6.3.2.2 "Function calls":

   $ The order of evaluation of the function designator, the
   $ arguments, and subexpressions within the arguments is
   $ unspecified, ...
--

I sit surprised and corrected.  With every version of every C compiler on
every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
Manx C to E1 using Kai C++) the behavior has been the same for parameter
lists as for the comma operator in this respect.  Does this imply that even
the evaluation of a function pointer is itself undefined in terms of
ordering?

"Premature optimization is the root of all evil."
--D. E. Knuth, "Structured Programming with go to Statements"

Absolutely true.

However, optimization of well tested, and algorithmically validated kernel
code in a time critical section does not constitute premature optimization
as I'm sure Donald would agree.

I would be really interested to see how well the kernel compiler does at
optimizing out all of these new tmp variables in terms of the resulting
register utilization and performance.  At least an assembler code size
comparison is in order for each of these individual cases.

That is my only real concern (other than all of that syntactically ambiguous
code I have written over the last 15 years ;-) is that critical performance
not be compromised in the kernel.

--Jonathan--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



RE: Patch to remove undefined C code

2000-10-16 Thread Jonathan George

-Original Message-
From: Alexander Viro [mailto:[EMAIL PROTECTED]]
[snip]

No arguments here, but proposed fixes were remarkably ugly. Example:

tmp = *p++;
*q = f(tmp, *p++);
return p;

is equivalent to more idiomatic

*q = f(p[0], p[1]);
return p+2;

And example with copying the string up to the comma... Yuck. Legal C !=
decent C.

Strongly agree.  If the changes were an elegant solution to the ambiguities
I never would have cared.  In fact the whole reason I bothered to read the
patch in the first place was that I feel strongly that something like that
_was_ needed, but the solutions really seemed inferior to the original code
in terms of elegance of expression. (Well some of the original code was
pretty ugly too :-)

--Jonathan--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Jonathan George [EMAIL PROTECTED] writes:

  $ The order of evaluation of the function designator, the
  $ arguments, and subexpressions within the arguments is
  $ unspecified, ...
 --
 
 I sit surprised and corrected.  With every version of every C compiler on
 every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
 Manx C to E1 using Kai C++) the behavior has been the same for parameter
 lists as for the comma operator in this respect.

Yes.  In practice the usual question is whether the compiler will
evaluate the operands from left to right or from right to left,
but the compiler is within its rights to evaluate the operands in
any order it wants.

 Does this imply that even the evaluation of a function pointer
 is itself undefined in terms of ordering?

Yes.

 "Premature optimization is the root of all evil."
 --D. E. Knuth, "Structured Programming with go to Statements"
 
 Absolutely true.

Good sigmonster, have $TREAT.

 I would be really interested to see how well the kernel compiler does at
 optimizing out all of these new tmp variables in terms of the resulting
 register utilization and performance.  At least an assembler code size
 comparison is in order for each of these individual cases.

I agree with you and Al Viro that the proposed fixes were less
than desirable.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Mike Castle

On Mon, Oct 16, 2000 at 04:47:09PM -0400, Alexander Viro wrote:
 tmp = *p++;
 *q = f(tmp, *p++);
 return p;
 
 is equivalent to more idiomatic
 
 *q = f(p[0], p[1]);
 return p+2;


Which gets better assembler out of various versions of gcc?

mrc
-- 
   Mike Castle   Life is like a clock:  You can work constantly
  [EMAIL PROTECTED]  and be right all the time, or not work at all
www.netcom.com/~dalgoda/ and be right at least twice a day.  -- mrc
We are all of us living in the shadow of Manhattan.  -- Watchmen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Alexander Viro



On Mon, 16 Oct 2000, Mike Castle wrote:

 On Mon, Oct 16, 2000 at 04:47:09PM -0400, Alexander Viro wrote:
  tmp = *p++;
  *q = f(tmp, *p++);
  return p;
  
  is equivalent to more idiomatic
  
  *q = f(p[0], p[1]);
  return p+2;
 
 
 Which gets better assembler out of various versions of gcc?

On which platform? If it would be VAX - sure, autoincrement rocks,
but for something like x86 I would expect the second form to do better.
Besides, it's more readable and is harder to fsck up when you are
modifying the code.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Matti Aarnio

On Mon, Oct 16, 2000 at 04:55:08PM -0400, Ben Pfaff wrote:
 $ The order of evaluation of the function designator, the
 $ arguments, and subexpressions within the arguments is
 $ unspecified, ...
  
  I sit surprised and corrected.  With every version of every C compiler on
  every OS I have ever used (over 100 combinations from AmigaDOS 1.1 using
  Manx C to E1 using Kai C++) the behavior has been the same for parameter
  lists as for the comma operator in this respect.
 
 Yes.  In practice the usual question is whether the compiler will
 evaluate the operands from left to right or from right to left,
 but the compiler is within its rights to evaluate the operands in
 any order it wants.

  That depends mainly on question:  Does your stack grow up or down ?

  Machines where stack grows up are a bit rare, but they do exist.
  The CISC archetype, IBM S/360/370/390 has simple instruction to
  add a small positive offset value into pointer register, but not
  to substract it (nor have negative offsets). Thus most stackfull
  languages there have the stack growing up.  Doing downwards-growing
  stack setup requires 1 extra register use, and one extra instruction
  per frame entry, plus considerably more for parameter push.
  (But it was a great beast to run Fortran-IV which didn't need stack,
   just subfunction invocation record -- at a static location unique
   for each subfunction -- no recursion supported.)

  Does this imply that even the evaluation of a function pointer
  is itself undefined in terms of ordering?
 
 Yes.


   For things like lone pointer referral with pre/post inc/dec:
*p++
   it is well defined, but for things like:
*p++ + *p
   it is not.  (Will the second *p be evaluated before or after *p++
   post-increment ?)


/Matti Aarnio
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch to remove undefined C code

2000-10-16 Thread Ben Pfaff

Matti Aarnio [EMAIL PROTECTED] writes:

 On Mon, Oct 16, 2000 at 04:55:08PM -0400, Ben Pfaff wrote:
  Yes.  In practice the usual question is whether the compiler will
  evaluate the operands from left to right or from right to left,
  but the compiler is within its rights to evaluate the operands in
  any order it wants.
 
   That depends mainly on question:  Does your stack grow up or down ?

No it doesn't.  It depends mainly on whether the ABI for the
machine says that arguments are pushed onto the stack in
left-to-right or right-to-left order.  You could do either on
x86, for instance, and it has nothing to do with whether the x86
stack grows up or down.

   Does this imply that even the evaluation of a function pointer
   is itself undefined in terms of ordering?
  
  Yes.

For things like lone pointer referral with pre/post inc/dec:
   *p++
it is well defined, but for things like:
   *p++ + *p
it is not.  (Will the second *p be evaluated before or after *p++
post-increment ?)

Non sequitur: I see no function pointers being evaluated here.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/