Re: egcs 1.0.1 miscompiles Linux 2.0.33

Christof Petig (christof.petig@wtal.de)
Wed, 11 Mar 1998 08:27:51 +0100


Jeffrey A Law wrote:

> In message <34FE6DB6.4D2CCA22@wtal.de>you write:
> > Could you please explain the syntax of an earlyclobber (I never heard of it).
> > I would undergo the task of patching Linux-Kernel's asms, since I'd like to
> > stop the debate about an EGCS/Kernel mismatch. EGCS is a valuable project a nd
> > compiling a kernel should not delay using it.
> It's pretty simple.
>
> For example strstr would look something like this:
>
> extern inline char * strstr(const char * cs,const char * ct)
> {
> int __dummy1;
> int __dummy2;
> register char * __res;
> __asm__ __volatile__(
> "cld\n\t" "movl %6,%%edi\n\t"
> [...]
> "xorl %%eax,%%eax\n\t"
> "2:"
> :"=a" (__res), "=&c" (__dummy1), "=&S" (__dummy2) :"0" (0), "1" (0xfffff
> fff), "2" (cs),"g" (ct)
> :"dx","di");
> return __res;
> }

Hmmm, this code might be well readable and optimizable by a compiler. Any human
beeing would definitely struggle with the set but unused variables. (Note Linus'
arguments against the readability of this syntactic construction. I agree. Though I
don't agree that something which has been valid should remain valid.)

I would suggest
:"=a" (__res), "=&c" (void), "=&S" (void)
: "0" (0), "1" (0xffffffff), "2" (cs),"g" (ct)
:"dx","di");
as a clean and both human and compiler readable form. Though it would request a
change within the compiler?
It is much like
(void) mkdir("foo");
which is commonly accepted as the correct way to satisfy lint.

> Notice how cx and si are no longer in the clobber list. Instead there's
> a dummy output operand for each with is marked as an earlyclobber (=&)
> of the register class which contains cx and si.
>
> The inputs which must be allocated to cx and si are forced to match
> the outputs via the "1" and "2" constraints respectively.
>
> Note that even with that fix, this asm is still has the potential to
> cause problems, especially when compiling with -fno-strength-reduce.
>
> The problem is ax, cx, si, dx and di are explicitly used by the insn.
> This leaves just bx for reloading.
>
> The general operand "g" (ct) may require 2 reload regs since it might
> used an indexed address. Unfortunately only one reload register is
> considered available (bx).
>
> This is a problem with the asm, not gcc. Basically when compiling
> with -fno-strength-reduce there may not enough registers to handle
> this asm, depending on address of "ct".
>
> Changing the constraint for "ct" to be a register will not help since
> the compiler would still need to reload the value of ct into the
> register to satisfy the register constraint, which still may require
> 2 reload registers.
>
> You must either rewrite the asm to use one less register or not
> compile with -fno-strength-reduce (note -fno-strength-reduce should
> not be necessary for gcc-2.8 or egcs).
>
> In fact, its this exact situation that started this whole discussion;
> right now the compiler may silently generate incorrect code if it
> runs out of registers for an x86 asm that explicitly clobbers registers.

Thank you for this detailed explanation.Here an fatal error would be the best
solution. Nobody could disagree on this topic. If done fast it would stop this
thread.

> I've got a change which will cause the compiler to issue a fatal error
> anytime a clobber overlaps with an input operand or the address of
> any operand. This change is likely to expose various problems in
> x86 asm statements for Linux since overlapping inputs and outputs
> is relatively common.
>
> I've hesitated installing this change because it's going to cause
> so many compile time failures for code which currently exists in the
> Linux sources.

A warning would be nice, a fatal error should be reserved for incorrect code.

> One possibility would be to make the behavior conditional on a switch;
> the default behavior right now would be to mimic the old behavior.
> Once y'all have had time to fix the problems we could make the more
> strict asm checking the default behavior.

Also Ok for me.

Anyway if there is public demand I would fix the asms right now (using
earlyclobbers).

Regards
Christof

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu