On Tue, Feb 17, 2015 at 3:51 AM, Thomas Preud'homme wrote:
>> > - else if (REG_P (src)
>> > - && REGNO (src) >= FIRST_PSEUDO_REGISTER
>> > - && REGNO (src) != regno)
>> > - {
>> > - if (try_replace_reg (reg_used, src, insn))
>> > + else if (src_reg && REG_P (src_reg)
>> > + && REGNO (src_reg) >= FIRST_PSEUDO_REGISTER
>> > + && REGNO (src_reg) != regno
>> > + && try_replace_reg (reg_used, src_reg, insn))
>>
>> Likewise for the REG_P and ">= FIRST_PSEUDO_REGISTER" tests here
>> (with
>> the equivalent and IMHO preferable HARD_REGISTER_P test in
>> find_avail_set()).
>
> I'm not sure I follow you here. First, it seems to me that the equivalent
> test is rather REG_P && !HARD_REGISTER_P since here it checks if it's
> a pseudo register.
>
> Then, do you mean the test can be simply removed because of the
> REG_P && !HARD_REGISTER_P in hash_scan_set () called indirectly by
> compute_hash_table () when called in one_cprop_pass () before any
> cprop_insn ()? Or do you mean I should move the check in
> find_avail_set ()?
What I meant, is that I believe the tests are already done in
hash_scan_set and should be redundant in cprop_insn (i.e. the test can
be replaced with gcc_[checking_]assert).
I've attached a patch with some changes to it: introduce cprop_reg_p()
to get rid of all the "REG_P && regno > FIRST_PSEUDO_REGISTER" tests.
I still have the cprop_constant_p and cprop_reg_p tests in cprop_insn
but this weekend I'll try with gcc_checking_asserts instead. Please
have a look at the patch and let me know if you like it (given it's
mostly yours I hope you do like it ;-)
Bootstrapped & tested on powerpc64-unknown-linux-gnu. In building all
of cc1, 35 extra copies are propagated with the patch.
Ciao!
Steven
Index: cprop.c
===================================================================
--- cprop.c (revision 221523)
+++ cprop.c (working copy)
@@ -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 has
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 has
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 @@ try_replace_reg (rtx from, rtx to, 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 @@ try_replace_reg (rtx from, rtx to, 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,7 +1055,8 @@ cprop_insn (rtx_insn *insn)
int changed = 0, changed_this_round;
rtx note;
-retry:
+ do
+ {
changed_this_round = 0;
reg_use_count = 0;
note_uses (&PATTERN (insn), find_used_regs, NULL);
@@ -1064,8 +1070,8 @@ cprop_insn (rtx_insn *insn)
{
rtx reg_used = reg_use_table[i];
unsigned int regno = REGNO (reg_used);
- rtx src;
- struct cprop_expr *set;
+ rtx src_cst = NULL, src_reg = NULL;
+ struct cprop_expr *set[2];
/* If the register has already been set in this block, there's
nothing we can do. */
@@ -1074,17 +1080,16 @@ cprop_insn (rtx_insn *insn)
/* 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;
+ find_avail_set (regno, insn, set);
+ if (set[0])
+ src_reg = set[0]->src;
+ if (set[1])
+ src_cst = set[1]->src;
- src = set->src;
-
/* Constant propagation. */
- if (cprop_constant_p (src))
+ if (src_cst && cprop_constant_p (src_cst)
+ && constprop_register (reg_used, src_cst, insn))
{
- if (constprop_register (reg_used, src, insn))
- {
changed_this_round = changed = 1;
global_const_prop_count++;
if (dump_file != NULL)
@@ -1093,19 +1098,17 @@ cprop_insn (rtx_insn *insn)
"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)
+ /* Copy propagation. */
+ else if (src_reg && cprop_reg_p (src_reg)
+ && REGNO (src_reg) != regno
+ && try_replace_reg (reg_used, src_reg, insn))
{
- if (try_replace_reg (reg_used, src, insn))
- {
changed_this_round = changed = 1;
global_copy_prop_count++;
if (dump_file != NULL)
@@ -1113,7 +1116,7 @@ cprop_insn (rtx_insn *insn)
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 @@ cprop_insn (rtx_insn *insn)
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;
- }
+ 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. */
