target/5169: Bug in pa-risc gcc optimizer

target/5169: Bug in pa-risc gcc optimizer

[Klaus.k.pedersen]

>Number:         5169
>Category:       target
>Synopsis:       Bug in pa-risc gcc optimizer
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          wrong-code
>Submitter-Id:   net
>Arrival-Date:   Fri Dec 21 05:26:01 PST 2001
>Closed-Date:
>Last-Modified:
>Originator:     Klaus Pedersen
>Release:        all - at least from 2.95 and up
>Organization:
>Environment:
HP-UX 11, HP-UX 10.20, gcc 2.95.2, to current snapshot 

For example
gcc -v
Reading specs from /opt/net/local/gcc302/lib/gcc-lib/hppa1.1-hp-hpux11.00/3.0.2/specs
Configured with: ../configure --enable-haifa --with-gnu-as --with-included-gettext --enable-shared --enable-languages=c,c++,f77 --enable-threads=posix --prefix=/opt/net/local/gcc302 --with-local-prefix=/opt/net/local/gcc302
Thread model: posix
gcc version 3.0.2

I have tried a wide selection of HP boxes (thanks to rodrigc@gcc.gnu.org,
who claims that he doesn't have any problems on HP). - from personal
workstations:
HP-UX coicxxx B.11.00 A 9000/785 2014470784 two-user license 
to N-class servers:
HP-UX caswxxx B.11.00 U 9000/800 600309332 unlimited-user license

>Description:
See also bootstrap/3479, closed by rodrigc@gcc.gnu.org
See also target/5107, investigated by Richard.Earnshaw@arm.com

The attached program, which is the reason that it is
impossible to build a cross compiler for ARM on HP-UX
with default CFLAGS=-O2.

Here is the program in all its glory:

----
int main()
{
    int operands[10] = {0, 65536};
    unsigned int val = operands[1];
    unsigned int mask = 0xff;
    int i;

    for (i = 0; i < 25; i++)
      if ((val & (mask << i)) == val)
        break;

    if (i == 0)
      puts("Fail");

    return 0;
}
----

The problem seem to be that "if (i==0)" is always true!

>How-To-Repeat:
Compile and run attached program:
   gcc -O1 hp-bug-1.c
   ./a.out
Everything is OK.

When compiling with -O2:
    gcc -O2 hp-bug-1.c
    ./a.out
the program returns "fail".
>Fix:
Never compile with higher optimizer settings than -O1 :-(
>Release-Note:
>Audit-Trail:
>Unformatted:
----gnatsweb-attachment----
Content-Type: application/x-unknown-content-type-c_auto_file; name="hp-bug-1.c"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="hp-bug-1.c"

aW50IG1haW4oKQp7CiAgICBpbnQgb3BlcmFuZHNbMTBdID0gezAsIDY1NTM2fTsKICAgIHVuc2ln
bmVkIGludCB2YWwgPSBvcGVyYW5kc1sxXTsKICAgIHVuc2lnbmVkIGludCBtYXNrID0gMHhmZjsK
ICAgIGludCBpOwogICAgCiAgICBmb3IgKGkgPSAwOyBpIDwgMjU7IGkrKykKICAgICAgaWYgKCh2
YWwgJiAobWFzayA8PCBpKSkgPT0gdmFsKQogICAgICAgIGJyZWFrOwoKICAgIGlmIChpID09IDAp
CiAgICAgIHB1dHMoIkZhaWwiKTsKCiAgICByZXR1cm4gMDsKfQo=

Re: target/5169: Bug in pa-risc gcc optimizer

[law]

The following reply was made to PR target/5169; it has been noted by GNATS.

From: law@redhat.com
To: Klaus Pedersen <klaus.k.pedersen@nokia.com>
Cc: ext Alan Modra <amodra@bigpond.net.au>, gcc-bugs@gcc.gnu.org,
        luke@stat.umn.edu, gcc-gnats@gcc.gnu.org, Richard.Earnshaw@arm.com,
        gcc-patches@gcc.gnu.org, rodrigc@gcc.gnu.org
Subject: Re: target/5169: Bug in pa-risc gcc optimizer 
Date: Thu, 03 Jan 2002 11:51:08 -0700

   > Thanks, your patch solves all of my problems! That means:
   > 
   >     target/5169: Bug in pa-risc gcc optimizer
   >     target/5185: Segmentation error in final.c
   > 
   > and as a bonus also this one:
   > 
   >     optimization/5264: Possible optimization bug at -O2 on HP-UX
   > 
   > 
   > With your patch applied to the snapshot gcc-20011231 I can bootstrap 
   > a working compiler gcc 2.95.3:
   > 
   >   ../gcc-20011231/configure --prefix=/... --enable-languages=c,c++ 
   >     --with-gnu-as --with-as=/opt/net/local/bin/as
   >   gmake bootstrap CFLAGS=-O1
   > 
   > Please note that if you are bootstrapping from gcc, then it is essencial
   > to make the first step of the bootstrap with optimizer level set to 
   > something lower than 2 (-O1 works).
   > 
   > With the native HP compiler this is probably not nessary.
   > 
   > Thanks, your patch might just solve years of mysterious problems! Now I
   > just 
   > have to build xemacs, to see if this solves the strange craches.
 Be very very careful thinking all is well with this problem, particularly
 if you're working with the current development sources.
 
 First, the testcase mentioned in the PR does not correspond to the RTL
 fragments Alan posted.  How do I know this?  Well, the testcase as a local
 variable named "operands".  Local variables are in registers or in stack
 slots.
 
 However, the RTL in the bug shows this:
 
 > (insn 12 10 15 (set (reg/v:SI 94)
 >         (mem/s:SI (lo_sum:SI (reg/f:SI 95)
 >                 (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                         (const_int 4 [0x4])))) 2)) 68 {*pa.md:2088}
 
 That would be consistent with operands being declared as a "static int", not
 an automatic variable.
 
 This (of course) was rather confusing when I was trying to double-check
 Alan's analysis.  Luckily after further investigation I can confirm that
 while the RTL and testcase don't match it's not that important (except for
 the poor bastard trying to reproduce Alan's results).
 
 To further confuse things, the bug is currently masked by unrelated changes
 in the compiler (code which tries to normalize the order of arguments in
 the comparison to make later code simpler).  
 
 
 jeff
 

Re: target/5169: Bug in pa-risc gcc optimizer

[Klaus Pedersen]

The following reply was made to PR target/5169; it has been noted by GNATS.

From: Klaus Pedersen <klaus.k.pedersen@nokia.com>
To: ext Alan Modra <amodra@bigpond.net.au>, gcc-bugs@gcc.gnu.org,
   luke@stat.umn.edu
Cc: gcc-gnats@gcc.gnu.org, Richard.Earnshaw@arm.com, gcc-patches@gcc.gnu.org,
   rodrigc@gcc.gnu.org
Subject: Re: target/5169: Bug in pa-risc gcc optimizer
Date: Thu, 03 Jan 2002 16:48:32 +0100

 Thanks, your patch solves all of my problems! That means:
 
     target/5169: Bug in pa-risc gcc optimizer
     target/5185: Segmentation error in final.c
 
 and as a bonus also this one:
 
     optimization/5264: Possible optimization bug at -O2 on HP-UX
 
 
 With your patch applied to the snapshot gcc-20011231 I can bootstrap 
 a working compiler gcc 2.95.3:
 
   ../gcc-20011231/configure --prefix=/... --enable-languages=c,c++ 
     --with-gnu-as --with-as=/opt/net/local/bin/as
   gmake bootstrap CFLAGS=-O1
 
 Please note that if you are bootstrapping from gcc, then it is essencial
 to make the first step of the bootstrap with optimizer level set to 
 something lower than 2 (-O1 works).
 
 With the native HP compiler this is probably not nessary.
 
 Thanks, your patch might just solve years of mysterious problems! Now I
 just 
 have to build xemacs, to see if this solves the strange craches.
 
 BR, Klaus
 
 
 
 Alan Modra wrote:
 > 
 > On Fri, Dec 21, 2001 at 01:19:37PM -0000, Klaus.k.pedersen@nokia.com
 > wrote:
 > >
 > > >Number:         5169
 > > >Category:       target
 > > >Synopsis:       Bug in pa-risc gcc optimizer
 > > >Confidential:   no
 > > >Severity:       serious
 > > >Priority:       medium
 > > >Responsible:    unassigned
 > > >State:          open
 > > >Class:          wrong-code
 > > >Submitter-Id:   net
 > > >Arrival-Date:   Fri Dec 21 05:26:01 PST 2001
 > > >Closed-Date:
 > > >Last-Modified:
 > > >Originator:     Klaus Pedersen
 > > >Release:        all - at least from 2.95 and up
 > > >Organization:
 > > >Environment:
 > > HP-UX 11, HP-UX 10.20, gcc 2.95.2, to current snapshot
 > >
 > > >Description:
 > > See also bootstrap/3479, closed by rodrigc@gcc.gnu.org
 > > See also target/5107, investigated by Richard.Earnshaw@arm.com
 > >
 > > The attached program, which is the reason that it is
 > > impossible to build a cross compiler for ARM on HP-UX
 > > with default CFLAGS=-O2.
 > >
 > > Here is the program in all its glory:
 > >
 > > ----
 > > int main()
 > > {
 > >     int operands[10] = {0, 65536};
 > >     unsigned int val = operands[1];
 > >     unsigned int mask = 0xff;
 > >     int i;
 > >
 > >     for (i = 0; i < 25; i++)
 > >       if ((val & (mask << i)) == val)
 > >         break;
 > >
 > >     if (i == 0)
 > >       puts("Fail");
 > >
 > >     return 0;
 > > }
 > > ----
 > >
 > > The problem seem to be that "if (i==0)" is always true!
 > 
 > try_combine, presented with the following three instructions:
 > 
 > (gdb) p debug_rtx (i1)
 > 
 > (insn 12 10 15 (set (reg/v:SI 94)
 >         (mem/s:SI (lo_sum:SI (reg/f:SI 95)
 >                 (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                         (const_int 4 [0x4])))) 2)) 68 {*pa.md:2088}
 > (insn_list 10 (nil))
 >     (expr_list:REG_DEAD (reg/f:SI 95)
 >         (nil)))
 > 
 > (gdb) p debug_rtx (i2)
 > 
 > (insn 90 87 91 (set (reg:SI 109)
 >         (zero_extend:SI (subreg:QI (reg/v:SI 94) 0))) 131 {*pa.md:3348}
 > (insn_list 12 (nil))
 >     (nil))
 > 
 > (gdb) p debug_rtx (i3)
 > 
 > (jump_insn 91 90 103 (set (pc)
 >         (if_then_else (eq (reg:SI 109)
 >                 (reg/v:SI 94))
 >             (label_ref 47)
 >             (pc))) 44 {*pa.md:1497} (insn_list 90 (nil))
 >     (expr_list:REG_DEAD (reg:SI 109)
 >         (expr_list:REG_BR_PROB (const_int 3999 [0xf9f])
 >             (nil))))
 > 
 > substitutes for the "eq" in the "jump_insn" to get:
 > 
 > (eq (subreg:SI (mem/s:QI (plus:SI (lo_sum:SI (reg/f:SI 95)
 >                     (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                             (const_int 4 [0x4]))))
 >                 (const_int 3 [0x3])) 2) 0)
 >     (mem/s:SI (lo_sum:SI (reg/f:SI 95)
 >             (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                     (const_int 4 [0x4])))) 2))
 > 
 > A little further down the track, this code in simplify_comparison thinks
 > that the only interesting part of the "eq" operands is the mem:QI, and
 > decides that the operands are always equal.
 > 
 >   /* Now make any compound operations involved in this comparison.
 > Then,
 >      check for an outmost SUBREG on OP0 that is not doing anything or is
 >      paradoxical.  The latter case can only occur when it is known that
 > the
 >      "extra" bits will be zero.  Therefore, it is safe to remove the
 > SUBREG.
 >      We can never remove a SUBREG for a non-equality comparison because
 > the
 >      sign bit is in a different place in the underlying object.  */
 > 
 >   op0 = make_compound_operation (op0, op1 == const0_rtx ? COMPARE :
 > SET);
 >   op1 = make_compound_operation (op1, SET);
 > 
 >   if (GET_CODE (op0) == SUBREG && subreg_lowpart_p (op0)
 >       && GET_MODE_CLASS (GET_MODE (op0)) == MODE_INT
 >       && (code == NE || code == EQ)
 >       && ((GET_MODE_SIZE (GET_MODE (op0))
 >            > GET_MODE_SIZE (GET_MODE (SUBREG_REG (op0))))))
 >     {
 >       op0 = SUBREG_REG (op0);
 >       op1 = gen_lowpart_for_combine (GET_MODE (op0), op1);
 >     }
 > 
 > I don't believe that's really where we're going wrong.  The chain of
 > replacements is:
 >     zero_extend:SI (subreg:QI (reg:SI))
 >  -> zero_extend:SI (subreg:QI (mem:SI))
 >  -> zero_extend:SI (mem:QI)
 >  -> subreg:SI (mem:QI)
 > 
 > Seems to me that the last replacement is the real problem.
 > expand_compound_operation does
 > 
 >   if (modewidth + len >= pos)
 >     tem = simplify_shift_const (NULL_RTX, unsignedp ? LSHIFTRT :
 > ASHIFTRT,
 >                                 GET_MODE (x),
 >                                 simplify_shift_const (NULL_RTX, ASHIFT,
 >                                                       GET_MODE (x),
 >                                                       XEXP (x, 0),
 >                                                       modewidth - pos -
 > len),
 >                                 modewidth - len);
 > 
 > and the outer simplify_shift_const gets called with
 > 
 > Breakpoint 7, simplify_shift_const (x=0x0, code=LSHIFTRT,
 > result_mode=SImode,
 >     varop=0x40176190, input_count=24) at
 > /src/parisc/gcc_new/gcc/combine.c:8970
 > (gdb) p debug_rtx (varop)
 > 
 > (ashift:SI (subreg:SI (mem/s:QI (plus:SI (lo_sum:SI (reg/f:SI 95)
 >                     (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                             (const_int 4 [0x4]))))
 >                 (const_int 3 [0x3])) 2) 0)
 >     (const_int 24 [0x18]))
 > 
 > The two shifts are recognized as being equivalent to an AND with 0xff,
 > and we eventually get to
 > 
 >   if (outer_op != NIL)
 >     {
 >       if (GET_MODE_BITSIZE (result_mode) < HOST_BITS_PER_WIDE_INT)
 >         outer_const = trunc_int_for_mode (outer_const, result_mode);
 > 
 >       if (outer_op == AND)
 >         x = simplify_and_const_int (NULL_RTX, result_mode, x,
 > outer_const);
 > 
 > #0  simplify_and_const_int (x=0x0, mode=SImode, varop=0x40176180,
 > constop=255)
 >     at /src/parisc/gcc_new/gcc/combine.c:7903
 > (gdb) p debug_rtx (varop)
 > (subreg:SI (mem/s:QI (plus:SI (lo_sum:SI (reg/f:SI 95)
 >                 (const:SI (plus:SI (symbol_ref:SI ("operands___0"))
 >                         (const_int 4 [0x4]))))
 >             (const_int 3 [0x3])) 2) 0)
 > 
 > Now, simplify_and_const_int figures that this expression doesn't have
 > any
 > nonzero bits outside 0xff because of the MEM mode, so there's no need
 > for
 > an AND at all.  Thus it returns (subreg:SI (mem:QI ...)), which taken in
 > isolation is a quite reasonable decision.  So, at a minimum, I think we
 > need something like the following:
 > 
 >         * combine.c (expand_compound_operation): Don't allow
 >         simplify_and_const_int to oversimplify zero_extend to a subreg.
 > 
 > --- combine.c~  Wed Nov  7 11:39:16 2001
 > +++ combine.c   Sat Dec 22 23:47:45 2001
 > @@ -5781,8 +5781,10 @@ expand_compound_operation (x)
 >      return x;
 > 
 >    /* If we couldn't do this for some reason, return the original
 > -     expression.  */
 > -  if (GET_CODE (tem) == CLOBBER)
 > +     expression.  Also, don't allow eg. (zero_extend:SI (mem:QI ...))
 > +     to be replaced by (subreg:SI (mem:QI ...)).  */
 > +  if (GET_CODE (tem) == CLOBBER
 > +      || (GET_CODE (tem) == SUBREG && XEXP (x, 0) == XEXP (tem, 0)))
 >      return x;
 > 
 >    return tem;
 > 
 > --
 > Alan Modra
 > IBM OzLabs - Linux Technology Centre