From: "H.J. Lu" <hjl.tools@gmail.com>
To: "Peter Bergner" <bergner@vnet.ibm.com>
Cc: "Richard Sandiford" <rsandifo@nildram.co.uk>,
gcc-patches <gcc-patches@gcc.gnu.org>,
"Jeff Law" <law@redhat.com>
Subject: Re: [PING H.J. Lu] Re: [PATCH] PR35371 GCSE loses track of REG_POINTER attribute
Date: Mon, 17 Mar 2008 14:32:00 -0000 [thread overview]
Message-ID: <6dc9ffc80803170654jf8469c7r5cad742e21f1d816@mail.gmail.com> (raw)
In-Reply-To: <20080312144656.GA4941@lucon.org>
[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]
Hi Peter,
I removed copyright update in your original patch and applied it on
gcc 4.4 at revision 133082. I didn't noticed any serious performance
regressions with SPEC CPU 2000/2006.
Thanks.
H.J.
On Wed, Mar 12, 2008 at 7:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Mar 10, 2008 at 10:32:13AM -0500, Peter Bergner wrote:
> > On Thu, 2008-02-28 at 17:08 -0600, Peter Bergner wrote:
> > > HJ,
> > >
> > > Given the x86/x86_64 issues with the last indexed load/store patch,
> > > can you SPEC test this patch to make sure the rtlanal.c change doesn't
> > > affect you? Thanks.
> >
> > HJ,
> >
> > If you get a chance, can you please SPEC test the patch located in:
> >
> > http://gcc.gnu.org/ml/gcc-patches/2008-02/msg01442.html
> >
> > just to make sure it doesn't have a negative impact on x86/x86_64?
> > Thanks.
>
> Hi Peter,
>
> I can use gcc to compile SPEC CPU now. But your patch won't apply
>
> patching file rtlanal.c
> patching file gcse.c
> Hunk #2 succeeded at 4463 (offset 7 lines).
> Hunk #4 succeeded at 6120 (offset 7 lines).
> patching file emit-rtl.c
> Reversed (or previously applied) patch detected! Assume -R? [n]
>
> against revision 133140. Do you have an updated patch?
>
> Thanks.
>
> H.J.
>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc-pr35371-2.patch --]
[-- Type: text/x-patch; name=gcc-pr35371-2.patch, Size: 6443 bytes --]
On Tue, 2008-02-26 at 12:25 -0700, Jeff Law wrote:
> If someone wanted to get real ambitious they could revamp the
> REG_POINTER propagation code as well. It's amazingly simplistic
> at the moment (see regclass.c:reg_scan_mark_refs). Basically it
> fails to propagate for any register destination that is set more
> than once, even if all the sets are of the proper form for
> propagating REG_POINTER.
Do you mean fix it up and then call it from more than just CSE?
Currently, the only call to reg_scan() isn't in a location that
will help me.
Anyway, I took Richard's advice and moved/renamed the new function
to emit-rtl.c. How does the new code look? For -O1 compiles, I had
to properly order the indexed load/store operands during expand, because
we never attempt to fix them up after that (actually, DSE seems to call
simplify, and swap_commutative_operands_p() correctly says we should
swap the operands, but for some reason I don't understand yet, DSE
seems to just throw away that result).
HJ,
Given the x86/x86_64 issues with the last indexed load/store patch,
can you SPEC test this patch to make sure the rtlanal.c change doesn't
affect you? Thanks.
Peter
--=-PIIVCxfVeqdczuah+k/I
Content-Disposition: attachment; filename=PR35371-3.diff
Content-Type: text/x-patch; name=PR35371-3.diff; charset=UTF-8
Content-Transfer-Encoding: 7bit
PR rtl-optimization/35371
* rtlanal.c: Update copyright year.
(commutative_operand_precedence): Give SYMBOL_REF's the same
precedence as REG_POINTER's and MEM_POINTER's.
* emit-rtl.c: Update copyright year.
(set_reg_attrs_from_value): Copy the REG_POINTER/MEM_POINTER
attribute over to the new reg rtx.
(gen_reg_rtx_copy): New function.
* gcse.c: Update copyright year.
(pre_delete): Call gen_reg_rtx_copy rather than gen_reg_rtx.
(hoist_code): Likewise.
(build_store_vectors): Likewise.
(delete_store): Likewise.
* loop-invariant.c: Update copyright year.
(move_invariant_reg): Call gen_reg_rtx_copy rather than gen_reg_rtx.
* rtl.h: Update copyright year.
(gen_reg_rtx_copy): Add prototype.
Index: rtlanal.c
===================================================================
--- rtlanal.c (revision 132568)
+++ rtlanal.c (working copy)
@@ -2898,6 +2898,8 @@ commutative_operand_precedence (rtx op)
switch (GET_RTX_CLASS (code))
{
case RTX_CONST_OBJ:
+ if (code == SYMBOL_REF)
+ return -1;
if (code == CONST_INT)
return -6;
if (code == CONST_DOUBLE)
Index: gcse.c
===================================================================
--- gcse.c (revision 132568)
+++ gcse.c (working copy)
@@ -4456,8 +4456,7 @@ pre_delete (void)
expressions into. Get the mode for the new pseudo from
the mode of the original destination pseudo. */
if (expr->reaching_reg == NULL)
- expr->reaching_reg
- = gen_reg_rtx (GET_MODE (SET_DEST (set)));
+ expr->reaching_reg = gen_reg_rtx_copy (SET_DEST (set));
gcse_emit_move_after (expr->reaching_reg, SET_DEST (set), insn);
delete_insn (insn);
@@ -4981,7 +4980,7 @@ hoist_code (void)
from the mode of the original destination pseudo. */
if (expr->reaching_reg == NULL)
expr->reaching_reg
- = gen_reg_rtx (GET_MODE (SET_DEST (set)));
+ = gen_reg_rtx_copy (SET_DEST (set));
gcse_emit_move_after (expr->reaching_reg, SET_DEST (set), insn);
delete_insn (insn);
@@ -6114,7 +6113,7 @@ build_store_vectors (void)
are any side effects. */
if (TEST_BIT (ae_gen[bb->index], ptr->index))
{
- rtx r = gen_reg_rtx (GET_MODE (ptr->pattern));
+ rtx r = gen_reg_rtx_copy (ptr->pattern);
if (dump_file)
fprintf (dump_file, "Removing redundant store:\n");
replace_store_insn (r, XEXP (st, 0), bb, ptr);
@@ -6437,7 +6436,7 @@ delete_store (struct ls_expr * expr, bas
rtx reg, i, del;
if (expr->reaching_reg == NULL_RTX)
- expr->reaching_reg = gen_reg_rtx (GET_MODE (expr->pattern));
+ expr->reaching_reg = gen_reg_rtx_copy (expr->pattern);
reg = expr->reaching_reg;
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (revision 132568)
+++ emit-rtl.c (working copy)
@@ -961,11 +961,32 @@ set_reg_attrs_from_value (rtx reg, rtx x
int offset;
offset = byte_lowpart_offset (GET_MODE (reg), GET_MODE (x));
- if (MEM_P (x) && MEM_OFFSET (x) && GET_CODE (MEM_OFFSET (x)) == CONST_INT)
- REG_ATTRS (reg)
- = get_reg_attrs (MEM_EXPR (x), INTVAL (MEM_OFFSET (x)) + offset);
- if (REG_P (x) && REG_ATTRS (x))
- update_reg_offset (reg, x, offset);
+ if (MEM_P (x))
+ {
+ if (MEM_OFFSET (x) && GET_CODE (MEM_OFFSET (x)) == CONST_INT)
+ REG_ATTRS (reg)
+ = get_reg_attrs (MEM_EXPR (x), INTVAL (MEM_OFFSET (x)) + offset);
+ if (MEM_POINTER (x))
+ mark_reg_pointer (reg, MEM_ALIGN (x));
+ }
+ else if (REG_P (x))
+ {
+ if (REG_ATTRS (x))
+ update_reg_offset (reg, x, offset);
+ if (REG_POINTER (x))
+ mark_reg_pointer (reg, REGNO_POINTER_ALIGN (REGNO (x)));
+ }
+}
+
+/* Generate a REG rtx for a new pseudo register, copying the mode
+ and attributes from X. */
+
+rtx
+gen_reg_rtx_copy (rtx x)
+{
+ rtx reg = gen_reg_rtx (GET_MODE (x));
+ set_reg_attrs_from_value (reg, x);
+ return reg;
}
/* Set the register attributes for registers contained in PARM_RTX.
Index: loop-invariant.c
===================================================================
--- loop-invariant.c (revision 132568)
+++ loop-invariant.c (working copy)
@@ -1193,7 +1193,7 @@ move_invariant_reg (struct loop *loop, u
need to create a temporary register. */
set = single_set (inv->insn);
dest = SET_DEST (set);
- reg = gen_reg_rtx (GET_MODE (dest));
+ reg = gen_reg_rtx_copy (dest);
/* Try replacing the destination by a new pseudoregister. */
if (!validate_change (inv->insn, &SET_DEST (set), reg, false))
Index: rtl.h
===================================================================
--- rtl.h (revision 132568)
+++ rtl.h (working copy)
@@ -1509,6 +1509,7 @@ extern rtvec gen_rtvec_v (int, rtx *);
extern rtx gen_reg_rtx (enum machine_mode);
extern rtx gen_rtx_REG_offset (rtx, enum machine_mode, unsigned int, int);
extern rtx gen_reg_rtx_offset (rtx, enum machine_mode, int);
+extern rtx gen_reg_rtx_copy (rtx);
extern rtx gen_label_rtx (void);
extern rtx gen_lowpart_common (enum machine_mode, rtx);
--=-PIIVCxfVeqdczuah+k/I--
prev parent reply other threads:[~2008-03-17 13:55 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-25 22:30 Peter Bergner
2008-02-25 23:13 ` Peter Bergner
2008-02-26 5:25 ` [PATCH,updated] " Peter Bergner
2008-02-26 5:49 ` [PATCH,withdrawn] " Peter Bergner
2008-02-26 18:00 ` [PATCH] " Richard Sandiford
2008-02-26 19:04 ` Peter Bergner
2008-02-26 19:45 ` Richard Sandiford
2008-02-26 20:06 ` Jeff Law
2008-02-29 1:32 ` Peter Bergner
2008-03-03 19:17 ` Jeff Law
2008-03-03 19:42 ` Peter Bergner
2008-03-03 20:55 ` Jeff Law
2008-03-10 15:32 ` [PING H.J. Lu] " Peter Bergner
2008-03-10 16:22 ` H.J. Lu
2008-03-12 14:48 ` H.J. Lu
2008-03-17 14:32 ` H.J. Lu [this message]
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=6dc9ffc80803170654jf8469c7r5cad742e21f1d816@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=bergner@vnet.ibm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=rsandifo@nildram.co.uk \
/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: link
Be 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).