public inbox for gcc-prs@sourceware.org help / color / mirror / Atom feed
From: Klaus Pedersen <klaus.k.pedersen@nokia.com> To: nobody@gcc.gnu.org Cc: gcc-prs@gcc.gnu.org, Subject: Re: target/5169: Bug in pa-risc gcc optimizer Date: Thu, 03 Jan 2002 07:56:00 -0000 [thread overview] Message-ID: <20020103155600.8245.qmail@sources.redhat.com> (raw) 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
next reply other threads:[~2002-01-03 15:56 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2002-01-03 7:56 Klaus Pedersen [this message] -- strict thread matches above, loose matches on Subject: below -- 2002-01-03 10:56 law 2001-12-21 5:26 Klaus.k.pedersen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20020103155600.8245.qmail@sources.redhat.com \ --to=klaus.k.pedersen@nokia.com \ --cc=gcc-prs@gcc.gnu.org \ --cc=nobody@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).