This is the mail archive of the
gcc-prs@gcc.gnu.org
mailing list for the GCC project.
Re: target/5169: Bug in pa-risc gcc optimizer
- From: Klaus Pedersen <klaus dot k dot pedersen at nokia dot com>
- To: nobody at gcc dot gnu dot org
- Cc: gcc-prs at gcc dot gnu dot org,
- Date: 3 Jan 2002 15:56:00 -0000
- Subject: Re: target/5169: Bug in pa-risc gcc optimizer
- Reply-to: Klaus Pedersen <klaus dot k dot pedersen at nokia dot com>
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