Re: pre egcs-1.1 testing and Linux 2.1.x

Andrea Arcangeli (arcangeli@mbox.queen.it)
Mon, 24 Aug 1998 09:26:51 +0200 (CEST)


On Sun, 23 Aug 1998, David S. Miller wrote:

> + optimize = 0;
> if (optimize > 0)
> TIMEVAR (combine_time, combine_instructions (insns, max_reg_num ()));
> + optimize = 1;
>
> /* Dump rtl code after insn combination. */
>
>This turns off certain optimizations entirely during the combine
>phase, plus you always reset it to being on, to a fixed value on top
>of that.

That' s a very lazy patch. It say you only that the bug is between the two
optimize change. The right patch would be:

if (regparm > 0)
{
old_optimize = optimize;
optimize = 0;
}

if (optimize)
buggy_optimization();

if (regparm > 0)
optimize = old_optimize;

>Combine does not check the variable 'optimize' anyways, most of the
>compiler works by checking specific flags. So the effect you are
>obtaining is that some other piece of code is checking optimize for a
>non-zero value and acting differently.

combine is not executed if optimize is 0.

>1) No comments explaining what the real intended effect of your change
> is.

It sound to me obvious. There are some buggy optimization in the function
I have pratically commented out.

>2) No ChangeLog entry, explaining exactly what the results your
> patches are obtaining, and precisely what is being worked around

The patches make -mregparm=3 more stable (hopefully perfect).

> and/or disabled in the compiler for correct operation.
>
> --- /tmp/gcc-2.8.1/loop.c Fri Feb 6 20:23:34 1998
> +++ gcc-2.8.1/loop.c Thu Jul 9 17:08:55 1998
> @@ -3037,6 +3037,7 @@
> register int count = 0;
> register rtx dest;
>
> + return;
> bzero ((char *) last_set, nregs * sizeof (rtx));
> for (insn = from; insn != to; insn = NEXT_INSN (insn))
> {
>
>You're disabling an entire class of optimizations in the loop
>optimizer. Maybe this fixes a specific problem on ix86, but you don't

Yes, because I had not the time to understand how RTL works to fix it
myself in the loop optimization code.

>mention what that is or why this makes a difference. Certainly this

It fix -mregparm=3 for i386 (that is the only issue of this thread).

>change should not happen on all other architectures too.

This change should _not_ happen everywhere. The bugs should be fixed in
the right place to allow optimization also when compiling with
-mregparm=3.

>
> I think that gcc should be:
>
> 1. realiable
> 2. efficient and optimized
>
>I think you are asking the egcs maintainers to look at this set of
>patches which disable a whole slew of optimizations and for them do
>all the work of finding out exactly what is the cause of things going
>wrong. The patch looks like it was a matter of "let's try disabling

Yes.

>this and that, and see what works". This is useful for narrowing down
>the cause of the bug, but to use it as the direct result for the final
>patch, no way.

I 100% agree, I never asked anybody to apply it. This patch show only that
the bug is in the optimization code and which is that code.

>As it stands you've left the maintainer who looks at your patch with a
>lot of work to do.

Exactly. I' d like to do it myself but he should be more fast than me
since I have also to understand gcc internals before start to fix the bug.

Andrea[s] Arcangeli

-
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.altern.org/andrebalsa/doc/lkml-faq.html