Re: BUG: wrong compilation of simple ANSI C program (gcc 2.96 CVS from 20000218)

[Zack Weinberg <zack at wolery dot cumb dot org>]

On Mon, Feb 21, 2000 at 03:01:32PM +0100, Daniel Lauer wrote:
> Hallo,
> 
> the compiler
> 	GNU C version 2.96 (CVS version from Feb 18, 2000)
> makes wrong assembler code when compiling the following C-Code with -O1.

...
> unsigned int buggy (unsigned int *param) {
>     unsigned int accu, zero=0, borrow;
>     accu    = - *param;
>     borrow  = - (accu > zero);

[translated to...]

>         negl    %eax            	eax := 0 - *param	    2^32 - 1
>         setbe   %bl             	bl  := ( 0 <= *param )	    1
>         decl    %ebx            	ebx--			    0

What's happened here is it's tried to rewrite the expression as 

     borrow = (accu <= zero) - 1;

which is exactly the same, but can be implemented with a setxx/dec
pair.  Then it's tried to merge the comparison against zero with the
negation of accu, which doesn't work.  Why?

From the ia32 Programmer's Reference Manual:

NEG   r/m32  Two's complement negate r/m32

Flags Affected
The CF flag is cleared to 0 if the source operand is 0; otherwise it
is set to 1. The OF, SF, ZF, AF, and PF flags are set according to the
result.

and

SETBE r/m8   Set byte if below or equal (CF=1 or ZF=1)

In other words, a negated value will always appear to be <= 0 when
tested with SETBE, because either the carry or the zero flags will be
true.

Switching to RTL:  the troublesome sequence is, unsurprisingly,
created by combine.  After flow1 we have:

(parallel[ 
   (set (reg/v:SI 27)
        (neg:SI (reg:SI 30)))
   (clobber (reg:CC 17 flags))
])

(set (reg:SI 36)
     (const_int 0))

(set (reg:CC 17 flags)
     (compare:CC (reg/v:SI 27)
                 (const_int 0 [0x0])))

(set (strict_low_part (subreg:QI (reg:SI 36) 0))
     (leu:QI (reg:CC 17 flags)
             (const_int 0)))

and after combine we have

(set (reg:SI 36)
     (const_int 0 [0x0]))

(parallel[
   (set (reg:CCNO 17 flags)
        (compare:CCNO (neg:SI (reg:SI 30))
                      (const_int 0)))
   (set (reg/v:SI 27)
        (neg:SI (reg:SI 30)))
])

(set (strict_low_part (subreg:QI (reg:SI 36) 0))
     (leu:QI (reg:CCNO 17 flags)
             (const_int 0)))

The band-aid fix is to disable the negsi2_cmpno and negsi2_cmp
patterns in i386.md.  The proper fix would be to add another CC mode
that accurately describes what NEG does to the flags.

Also, note that we could get this specific case right by recognizing
that, for unsigned values, (val <= 0) is equivalent to (val == 0).
This is for expand_expr or fold to do, I believe - the transform from 
-(val > 0) to (val <= 0) - 1 appeared in the initial RTL.  If we had
had

(set (strict_low_part (subreg:QI (reg:SI 36) 0))
     (eq:QI (reg:CCNO 17 flags)
            (const_int 0)))

at assembly output time, we would have gotten a SETE instead of a
SETGE instruction, which would work.

May I add your test case to the testsuite?

zw