This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: target/5169: Bug in pa-risc gcc optimizer
- From: Alan Modra <amodra at bigpond dot net dot au>
- To: Klaus dot k dot pedersen at nokia dot com
- Cc: gcc-gnats at gcc dot gnu dot org, Richard dot Earnshaw at arm dot com, gcc-patches at gcc dot gnu dot org
- Date: Sun, 23 Dec 2001 00:11:42 +1030
- Subject: Re: target/5169: Bug in pa-risc gcc optimizer
- References: <20011221131937.22345.qmail@sources.redhat.com>
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