From: "Thomas Preud'homme" <thomas.preudhomme@arm.com>
To: "Steven Bosscher" <stevenb.gcc@gmail.com>,
"'Jeff Law'" <law@redhat.com>, <gcc-patches@gcc.gnu.org>,
"'Richard Biener'" <rguenther@suse.de>
Subject: RE: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible
Date: Thu, 16 Apr 2015 08:44:00 -0000 [thread overview]
Message-ID: <000001d07821$6fb82f60$4f288e20$@arm.com> (raw)
In-Reply-To: <552BBAF9.2010504@redhat.com>
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Monday, April 13, 2015 8:48 PM
>
> I know there were several followups between Steven and yourself.
> With
> stage1 now open, can you post a final version and do a final
> bootstrap/test with it?
Here is what came out of our discussion with Steven:
The RTL cprop pass in GCC operates by doing a local constant/copy propagation
first and then a global one. In the local one, if a constant cannot be propagated
(eg. due to constraints of the destination instruction) a copy propagation is
done instead. However, at the global level copy propagation is only tried if no
constant can be propagated, ie. if a constant can be propagated but the
constraints of the destination instruction forbids it, no copy propagation will be
tried. This patch fixes this issue.
ChangeLog entries are as follows:
*** gcc/ChangeLog ***
2015-04-15 Thomas Preud'homme <thomas.preudhomme@arm.com>
Steven Bosscher <stevenb.gcc@gmail.com>
* cprop.c (cprop_reg_p): New.
(hash_scan_set): Use above function to check if register can be
propagated.
(find_avail_set): Return up to two sets, one whose source is
a register and one whose source is a constant. Sets are returned in
an array passed as parameter rather than as a return value.
(cprop_insn): Use a do while loop rather than a goto. Try each of the
sets returned by find_avail_set, starting with the one whose source is
a constant. Use cprop_reg_p to check if register can be propagated.
(do_local_cprop): Use cprop_reg_p to check if register can be
propagated.
(implicit_set_cond_p): Likewise.
*** gcc/testsuite/ChangeLog ***
2015-04-15 Thomas Preud'homme <thomas.preudhomme@arm.com>
Steven Bosscher <stevenb.gcc@gmail.com>
* gcc.target/arm/pr64616.c: New file.
And the patch is:
diff --git a/gcc/cprop.c b/gcc/cprop.c
index c9fb2fc..78541cf 100644
--- a/gcc/cprop.c
+++ b/gcc/cprop.c
@@ -285,6 +285,15 @@ cprop_constant_p (const_rtx x)
return CONSTANT_P (x) && (GET_CODE (x) != CONST || shared_const_p (x));
}
+/* Determine whether the rtx X should be treated as a register that can
+ be propagated. Any pseudo-register is fine. */
+
+static bool
+cprop_reg_p (const_rtx x)
+{
+ return REG_P (x) && !HARD_REGISTER_P (x);
+}
+
/* Scan SET present in INSN and add an entry to the hash TABLE.
IMPLICIT is true if it's an implicit set, false otherwise. */
@@ -295,8 +304,7 @@ hash_scan_set (rtx set, rtx_insn *insn, struct hash_table_d *table,
rtx src = SET_SRC (set);
rtx dest = SET_DEST (set);
- if (REG_P (dest)
- && ! HARD_REGISTER_P (dest)
+ if (cprop_reg_p (dest)
&& reg_available_p (dest, insn)
&& can_copy_p (GET_MODE (dest)))
{
@@ -321,9 +329,8 @@ hash_scan_set (rtx set, rtx_insn *insn, struct hash_table_d *table,
src = XEXP (note, 0), set = gen_rtx_SET (VOIDmode, dest, src);
/* Record sets for constant/copy propagation. */
- if ((REG_P (src)
+ if ((cprop_reg_p (src)
&& src != dest
- && ! HARD_REGISTER_P (src)
&& reg_available_p (src, insn))
|| cprop_constant_p (src))
insert_set_in_table (dest, src, insn, table, implicit);
@@ -821,15 +828,15 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn)
return success;
}
-/* Find a set of REGNOs that are available on entry to INSN's block. Return
- NULL no such set is found. */
+/* Find a set of REGNOs that are available on entry to INSN's block. If found,
+ SET_RET[0] will be assigned a set with a register source and SET_RET[1] a
+ set with a constant source. If not found the corresponding entry is set to
+ NULL. */
-static struct cprop_expr *
-find_avail_set (int regno, rtx_insn *insn)
+static void
+find_avail_set (int regno, rtx_insn *insn, struct cprop_expr *set_ret[2])
{
- /* SET1 contains the last set found that can be returned to the caller for
- use in a substitution. */
- struct cprop_expr *set1 = 0;
+ set_ret[0] = set_ret[1] = NULL;
/* Loops are not possible here. To get a loop we would need two sets
available at the start of the block containing INSN. i.e. we would
@@ -869,8 +876,10 @@ find_avail_set (int regno, rtx_insn *insn)
If the source operand changed, we may still use it for the next
iteration of this loop, but we may not use it for substitutions. */
- if (cprop_constant_p (src) || reg_not_set_p (src, insn))
- set1 = set;
+ if (cprop_constant_p (src))
+ set_ret[1] = set;
+ else if (reg_not_set_p (src, insn))
+ set_ret[0] = set;
/* If the source of the set is anything except a register, then
we have reached the end of the copy chain. */
@@ -881,10 +890,6 @@ find_avail_set (int regno, rtx_insn *insn)
and see if we have an available copy into SRC. */
regno = REGNO (src);
}
-
- /* SET1 holds the last set that was available and anticipatable at
- INSN. */
- return set1;
}
/* Subroutine of cprop_insn that tries to propagate constants into
@@ -1050,40 +1055,40 @@ cprop_insn (rtx_insn *insn)
int changed = 0, changed_this_round;
rtx note;
-retry:
- changed_this_round = 0;
- reg_use_count = 0;
- note_uses (&PATTERN (insn), find_used_regs, NULL);
-
- /* We may win even when propagating constants into notes. */
- note = find_reg_equal_equiv_note (insn);
- if (note)
- find_used_regs (&XEXP (note, 0), NULL);
-
- for (i = 0; i < reg_use_count; i++)
+ do
{
- rtx reg_used = reg_use_table[i];
- unsigned int regno = REGNO (reg_used);
- rtx src;
- struct cprop_expr *set;
+ changed_this_round = 0;
+ reg_use_count = 0;
+ note_uses (&PATTERN (insn), find_used_regs, NULL);
- /* If the register has already been set in this block, there's
- nothing we can do. */
- if (! reg_not_set_p (reg_used, insn))
- continue;
+ /* We may win even when propagating constants into notes. */
+ note = find_reg_equal_equiv_note (insn);
+ if (note)
+ find_used_regs (&XEXP (note, 0), NULL);
- /* Find an assignment that sets reg_used and is available
- at the start of the block. */
- set = find_avail_set (regno, insn);
- if (! set)
- continue;
+ for (i = 0; i < reg_use_count; i++)
+ {
+ rtx reg_used = reg_use_table[i];
+ unsigned int regno = REGNO (reg_used);
+ rtx src_cst = NULL, src_reg = NULL;
+ struct cprop_expr *set[2];
- src = set->src;
+ /* If the register has already been set in this block, there's
+ nothing we can do. */
+ if (! reg_not_set_p (reg_used, insn))
+ continue;
- /* Constant propagation. */
- if (cprop_constant_p (src))
- {
- if (constprop_register (reg_used, src, insn))
+ /* Find an assignment that sets reg_used and is available
+ at the start of the block. */
+ find_avail_set (regno, insn, set);
+ if (set[0])
+ src_reg = set[0]->src;
+ if (set[1])
+ src_cst = set[1]->src;
+
+ /* Constant propagation. */
+ if (src_cst && cprop_constant_p (src_cst)
+ && constprop_register (reg_used, src_cst, insn))
{
changed_this_round = changed = 1;
global_const_prop_count++;
@@ -1093,18 +1098,16 @@ retry:
"GLOBAL CONST-PROP: Replacing reg %d in ", regno);
fprintf (dump_file, "insn %d with constant ",
INSN_UID (insn));
- print_rtl (dump_file, src);
+ print_rtl (dump_file, src_cst);
fprintf (dump_file, "\n");
}
if (insn->deleted ())
return 1;
}
- }
- else if (REG_P (src)
- && REGNO (src) >= FIRST_PSEUDO_REGISTER
- && REGNO (src) != regno)
- {
- if (try_replace_reg (reg_used, src, insn))
+ /* Copy propagation. */
+ else if (src_reg && cprop_reg_p (src_reg)
+ && REGNO (src_reg) != regno
+ && try_replace_reg (reg_used, src_reg, insn))
{
changed_this_round = changed = 1;
global_copy_prop_count++;
@@ -1113,7 +1116,7 @@ retry:
fprintf (dump_file,
"GLOBAL COPY-PROP: Replacing reg %d in insn %d",
regno, INSN_UID (insn));
- fprintf (dump_file, " with reg %d\n", REGNO (src));
+ fprintf (dump_file, " with reg %d\n", REGNO (src_reg));
}
/* The original insn setting reg_used may or may not now be
@@ -1123,12 +1126,10 @@ retry:
and made things worse. */
}
}
-
- /* If try_replace_reg simplified the insn, the regs found
- by find_used_regs may not be valid anymore. Start over. */
- if (changed_this_round)
- goto retry;
}
+ /* If try_replace_reg simplified the insn, the regs found by find_used_regs
+ may not be valid anymore. Start over. */
+ while (changed_this_round);
if (changed && DEBUG_INSN_P (insn))
return 0;
@@ -1191,7 +1192,7 @@ do_local_cprop (rtx x, rtx_insn *insn)
/* Rule out USE instructions and ASM statements as we don't want to
change the hard registers mentioned. */
if (REG_P (x)
- && (REGNO (x) >= FIRST_PSEUDO_REGISTER
+ && (cprop_reg_p (x)
|| (GET_CODE (PATTERN (insn)) != USE
&& asm_noperands (PATTERN (insn)) < 0)))
{
@@ -1207,7 +1208,7 @@ do_local_cprop (rtx x, rtx_insn *insn)
if (cprop_constant_p (this_rtx))
newcnst = this_rtx;
- if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER
+ if (cprop_reg_p (this_rtx)
/* Don't copy propagate if it has attached REG_EQUIV note.
At this point this only function parameters should have
REG_EQUIV notes and if the argument slot is used somewhere
@@ -1328,9 +1329,8 @@ implicit_set_cond_p (const_rtx cond)
if (GET_CODE (cond) != EQ && GET_CODE (cond) != NE)
return false;
- /* The first operand of COND must be a pseudo-reg. */
- if (! REG_P (XEXP (cond, 0))
- || HARD_REGISTER_P (XEXP (cond, 0)))
+ /* The first operand of COND must be a register we can propagate. */
+ if (!cprop_reg_p (XEXP (cond, 0)))
return false;
/* The second operand of COND must be a suitable constant. */
diff --git a/gcc/testsuite/gcc.target/arm/pr64616.c b/gcc/testsuite/gcc.target/arm/pr64616.c
new file mode 100644
index 0000000..c686ffa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr64616.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int f (int);
+unsigned int glob;
+
+void
+g ()
+{
+ while (f (glob));
+ glob = 5;
+}
+
+/* { dg-final { scan-assembler-times "ldr" 2 } } */
Best regards,
Thomas
next prev parent reply other threads:[~2015-04-16 8:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-16 10:26 Thomas Preud'homme
2015-02-16 10:54 ` Richard Biener
2015-02-16 12:06 ` Steven Bosscher
2015-02-16 20:20 ` Steven Bosscher
2015-02-17 2:51 ` Thomas Preud'homme
2015-03-04 8:52 ` Thomas Preud'homme
2015-03-20 7:55 ` Steven Bosscher
2015-03-20 8:36 ` Thomas Preud'homme
2015-03-20 10:27 ` Thomas Preud'homme
2015-03-20 12:14 ` Steven Bosscher
2015-03-23 11:01 ` Thomas Preud'homme
2015-03-23 11:57 ` Steven Bosscher
2015-03-30 4:58 ` Thomas Preud'homme
2015-04-13 12:47 ` Jeff Law
2015-04-14 8:00 ` Thomas Preud'homme
2015-04-16 8:44 ` Thomas Preud'homme [this message]
2015-04-23 9:15 ` Steven Bosscher
2015-04-24 2:59 ` Jeff Law
2015-04-24 3:11 ` Thomas Preud'homme
2015-04-24 3:15 ` Jeff Law
2015-04-24 4:53 ` Thomas Preud'homme
2015-04-30 7:43 ` Bin.Cheng
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='000001d07821$6fb82f60$4f288e20$@arm.com' \
--to=thomas.preudhomme@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=rguenther@suse.de \
--cc=stevenb.gcc@gmail.com \
/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).