From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21218 invoked by alias); 19 Aug 2005 22:53:01 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 21180 invoked by uid 22791); 19 Aug 2005 22:52:54 -0000 Received: from mail-out4.apple.com (HELO mail-out4.apple.com) (17.254.13.23) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Fri, 19 Aug 2005 22:52:54 +0000 Received: from mailgate2.apple.com (a17-128-100-204.apple.com [17.128.100.204]) by mail-out4.apple.com (8.12.11/8.12.11) with ESMTP id j7JMqqp1009649 for ; Fri, 19 Aug 2005 15:52:52 -0700 (PDT) Received: from relay2.apple.com (relay2.apple.com) by mailgate2.apple.com (Content Technologies SMTPRS 4.3.17) with ESMTP id for ; Fri, 19 Aug 2005 15:52:52 -0700 Received: from [17.201.20.185] (johada5.apple.com [17.201.20.185]) by relay2.apple.com (8.12.11/8.12.11) with ESMTP id j7JMqoGn028926 for ; Fri, 19 Aug 2005 15:52:51 -0700 (PDT) Mime-Version: 1.0 (Apple Message framework v622) To: gcc mailing list Message-Id: <263482fcfbcc31184df9e901a9811db9@apple.com> Content-Type: multipart/mixed; boundary=Apple-Mail-2-155898350 From: Dale Johannesen Subject: Fwd: [RFC] - Regression exposed by recent change to compress_float_constant Date: Fri, 19 Aug 2005 22:53:00 -0000 X-SW-Source: 2005-08/txt/msg00541.txt.bz2 --Apple-Mail-2-155898350 Content-Type: multipart/alternative; boundary=Apple-Mail-3-155898350 --Apple-Mail-3-155898350 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-length: 2694 Fariborz is still having problems with his mailer and has asked me to=20 forward this. On Aug 10, 2005, at 2:35 PM, Dale Johannesen wrote: > > On Aug 10, 2005, at 12:43 PM, Fariborz Jahanian wrote: > > >> Following patch has exposed an optimization shortcoming: >> >> 2005-07-12 Dale Johannesen >> >> =A0 * expr.c (compress_float_constant): Add cost check. >> =A0 * config/rs6000.c (rs6000_rtx_cost): Adjust FLOAT_EXTEND cost. >> >> This patch results in generating worse code for the following test=20 >> case: >> >> 1) Test case: >> >> struct S { >> =A0 float d1, d2, d3; >> > > I believe you mean double not float; the RTL snippets you give=20 > indicate this. > > >> (insn 12 7 13 0 (set (reg:SF 59) >> =A0 (mem/u/i:SF (symbol_ref/u:SI ("*LC0") [flags 0x2]) [0 S4 A32])) -1=20 >> (nil) >> =A0 (nil)) >> >> (insn 13 12 14 0 (set (mem/s/j:DF (reg/f:SI 58 [ D.1929 ]) [0=20 >> .d1+0 S8 A32]) >> =A0 (float_extend:DF (reg:SF 59))) -1 (nil) >> =A0 (nil)) >> > > However, if you try your example with float as given, you see it does=20 > not do a > direct store of constant 0 with or without the compress_float patch.=20 > IMO the > compress_float patch does not really have anything to do with this=20 > problem; > before this patch the double case was working well by accident, my=20 > patch > exposed a problem farther downstream, which was always there for the=20 > float > case. > > When I put that patch in, rth remarked: > > While I certainly wouldn't expect fold_rtx to find out about this > all by itself, I'd have thought that there would have been a > REG_EQUIV or REG_EQUAL note that indicates that the end result is > the constant (const_double:DF 1.0), and use that in any simplification. > > Indeed there is no such note, and I suspect adding it somewhere=20 > (expand?) would fix this. It turned out that cse does put REG_EQUIV on the insn which sets load=20 of "LC0" to the register. So, no need to do this. It also tells me that=20 cse is expected to use this information to do the constant propagation=20 (which in the example test case is the next few insns). Attached patch=20 accomplishes this task. It is against apple local branch. It has been=20 bootstrapped and dejagnu tested on x86-darwin, ppc-darwin. Note that=20 patch is similar to the code right before it (which is also shown in=20 this patch), so there is a precedence for this type of a fix. If this=20 looks reasonable, I will prepare an FSF patch. ChangeLog: 2005-08-19=A0 Fariborz Jahanian =A0 =A0 =A0 =A0 * cse.c (cse_insn): Use the constant to propagte =A0 =A0 =A0 =A0 into the rhs of a set insn which is a register. =A0 =A0 =A0 =A0 This is cheaper. --Apple-Mail-3-155898350 Content-Transfer-Encoding: quoted-printable Content-Type: text/enriched; charset=ISO-8859-1 Content-length: 3014 Fariborz is still having problems with his mailer and has asked me to forward this. On Aug 10, 2005, at 2:35 PM, Dale Johannesen wrote: On Aug 10, 2005, at 12:43 PM, Fariborz Jahanian wrote: Following patch has exposed an optimization shortcoming: 2005-07-12 Dale Johannesen <<0000,0000,EEEEdalej@apple.com> =A0 * expr.c (compress_float_constant): Add cost check. =A0 * config/rs6000.c (rs6000_rtx_cost): Adjust FLOAT_EXTEND cost. This patch results in generating worse code for the following test case: 1) Test case: struct S { =A0 float d1, d2, d3; I believe you mean double not float; the RTL snippets you give indicate this. (insn 12 7 13 0 (set (reg:SF 59) =A0 (mem/u/i:SF (symbol_ref/u:SI ("*LC0") [flags 0x2]) [0 S4 A32])) -1 (nil) =A0 (nil)) (insn 13 12 14 0 (set (mem/s/j:DF (reg/f:SI 58 [ D.1929 ]) [0 <.d1+0 S8 A32]) =A0 (float_extend:DF (reg:SF 59))) -1 (nil) =A0 (nil)) However, if you try your example with float as given, you see it does not do a=20 direct store of constant 0 with or without the compress_float patch. IMO the compress_float patch does not really have anything to do with this problem; before this patch the double case was working well by accident, my patch exposed a problem farther downstream, which was always there for the float case. When I put that patch in, rth remarked: CourierWhile I certainly wouldn't expect fold_rtx to find out about this Courierall by itself, I'd have thought that there would have been a CourierREG_EQUIV or REG_EQUAL note that indicates that the end result is Courierthe constant (const_double:DF 1.0), and use that in any simplification. Indeed there is no such note, and I suspect adding it somewhere (expand?) would fix this. It turned out that cse does put REG_EQUIV on the insn which sets load of "LC0" to the register. So, no need to do this. It also tells me that cse is expected to use this information to do the constant propagation (which in the example test case is the next few insns). Attached patch accomplishes this task. It is against apple local branch. It has been bootstrapped and dejagnu tested on x86-darwin, ppc-darwin. Note that patch is similar to the code right before it (which is also shown in this patch), so there is a precedence for this type of a fix. If this looks reasonable, I will prepare an FSF patch. ChangeLog: 2005-08-19=A0 Fariborz Jahanian <<0000,0000,EEEEfjahanian@apple.com> =A0 =A0 =A0 =A0 * cse.c (cse_insn): Use the constant to propagte =A0 =A0 =A0 =A0 into the rhs of a set insn which is a register. =A0 =A0 =A0 =A0 This is cheaper. --Apple-Mail-3-155898350-- --Apple-Mail-2-155898350 Content-Transfer-Encoding: 7bit Content-Type: text/plain; x-unix-mode=0644; name="radar-patch-4153339.txt" Content-Disposition: attachment; filename=radar-patch-4153339.txt Content-length: 2292 Index: cse.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cse.c,v retrieving revision 1.342.4.3 diff -c -p -r1.342.4.3 cse.c *** cse.c 5 Jul 2005 23:21:50 -0000 1.342.4.3 --- cse.c 19 Aug 2005 18:21:56 -0000 *************** cse_insn (rtx insn, rtx libcall_insn) *** 5455,5460 **** --- 5455,5469 ---- if (dest == pc_rtx && src_const && GET_CODE (src_const) == LABEL_REF) src_folded = src_const, src_folded_cost = src_folded_regcost = -1; + /* APPLE LOCAL begin radar 4153339 */ + if (n_sets == 1 && GET_CODE (sets[i].src) == REG + && src_const && GET_CODE (src_const) == CONST_DOUBLE) + { + src_folded = src_const; + src_folded_cost = src_folded_regcost = -1; + } + /* APPLE LOCAL end radar 4153339 */ + /* Terminate loop when replacement made. This must terminate since the current contents will be tested and will always be valid. */ while (1) Index: testsuite/ChangeLog.apple-ppc =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/Attic/ChangeLog.apple-ppc,v retrieving revision 1.1.4.88 diff -c -p -r1.1.4.88 ChangeLog.apple-ppc *** testsuite/ChangeLog.apple-ppc 15 Aug 2005 21:02:26 -0000 1.1.4.88 --- testsuite/ChangeLog.apple-ppc 19 Aug 2005 18:21:59 -0000 *************** *** 1,3 **** --- 1,8 ---- + 2005-08-18 Fariborz Jahanian + + Radar 4153339 + * gcc.dg/i386-movl-float.c: New. + 2005-08-15 Devang Patel Radar 4209318 Index: testsuite/gcc.dg/i386-movl-float.c =================================================================== RCS file: testsuite/gcc.dg/i386-movl-float.c diff -N testsuite/gcc.dg/i386-movl-float.c *** /dev/null 1 Jan 1970 00:00:00 -0000 --- testsuite/gcc.dg/i386-movl-float.c 19 Aug 2005 18:22:03 -0000 *************** *** 0 **** --- 1,15 ---- + /* APPLE LOCAL begin radar 4153339 */ + /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ + /* { dg-options "-O1 -mdynamic-no-pic -march=pentium4 -mtune=prescott" } */ + /* { dg-final { scan-assembler-times "movl\[^\\n\]*" 8} } */ + + struct S { + double d1, d2, d3; + }; + + struct S ms() + { + struct S s = {0,0,0}; + return s; + } + /* APPLE LOCAL end radar 4153339 */ --Apple-Mail-2-155898350 Content-Type: multipart/alternative; boundary=Apple-Mail-4-155898351 --Apple-Mail-4-155898351 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed Content-length: 1 --Apple-Mail-4-155898351 Content-Transfer-Encoding: 7bit Content-Type: text/enriched; charset=US-ASCII Content-length: 2 --Apple-Mail-4-155898351-- --Apple-Mail-2-155898350--