* [PATCH, loop2_invariant] Pre-check invariants
@ 2014-06-10 9:55 Zhenqiang Chen
2014-06-10 11:01 ` Steven Bosscher
0 siblings, 1 reply; 4+ messages in thread
From: Zhenqiang Chen @ 2014-06-10 9:55 UTC (permalink / raw)
To: gcc-patches
Hi,
During tests, I found some invariants could not be replaced at the
last stage. If we can identify such invariants earlier, we can skip
them and give the chance to other invariants. So the patch pre-checks
candidates to skip the one which can not make a valid insn during
replacement in move_invariant_reg.
Bootstrap and no make check regression on X86-64.
Bootstrap and no make check regression on X86-64 with
flag_ira_loop_pressure = true.
OK for trunk?
Thanks!
-Zhenqiang
ChangeLog:
2014-06-10 Zhenqiang Chen <zhenqiang.chen@linaro.org>
* loop-invariant.c (find_invariant_insn): Skip invariants, which
can not make a valid insn during replacement in move_invariant_reg.
diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index c43206a..7be4b29 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -881,6 +881,35 @@ find_invariant_insn (rtx insn, bool
always_reached, bool always_executed)
|| HARD_REGISTER_P (dest))
simple = false;
+ /* Pre-check candidate to skip the one which can not make a valid insn
+ during move_invariant_reg. */
+ if (flag_ira_loop_pressure && df_live && simple
+ && REG_P (dest) && DF_REG_DEF_COUNT (REGNO (dest)) > 1)
+ {
+ df_ref use;
+ rtx ref;
+ unsigned int i = REGNO (dest);
+ struct df_insn_info *insn_info;
+ df_ref *def_rec;
+
+ for (use = DF_REG_USE_CHAIN (i); use; use = DF_REF_NEXT_REG (use))
+ {
+ ref = DF_REF_INSN (use);
+ insn_info = DF_INSN_INFO_GET (ref);
+
+ for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)
+ if (DF_REF_REGNO (*def_rec) == i)
+ {
+ /* Multi definitions at this stage, most likely are due to
+ instruction constrain, which requires both read and write
+ on the same register. Since move_invariant_reg is not
+ powerful enough to handle such cases, just ignore the INV
+ and leave the chance to others. */
+ return;
+ }
+ }
+ }
+
if (!may_assign_reg_p (SET_DEST (set))
|| !check_maybe_invariant (SET_SRC (set)))
return;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, loop2_invariant] Pre-check invariants
2014-06-10 9:55 [PATCH, loop2_invariant] Pre-check invariants Zhenqiang Chen
@ 2014-06-10 11:01 ` Steven Bosscher
2014-06-11 9:35 ` Zhenqiang Chen
0 siblings, 1 reply; 4+ messages in thread
From: Steven Bosscher @ 2014-06-10 11:01 UTC (permalink / raw)
To: Zhenqiang Chen; +Cc: gcc-patches
On Tue, Jun 10, 2014 at 11:55 AM, Zhenqiang Chen wrote:
>
> * loop-invariant.c (find_invariant_insn): Skip invariants, which
> can not make a valid insn during replacement in move_invariant_reg.
>
> --- a/gcc/loop-invariant.c
> +++ b/gcc/loop-invariant.c
> @@ -881,6 +881,35 @@ find_invariant_insn (rtx insn, bool
> always_reached, bool always_executed)
> || HARD_REGISTER_P (dest))
> simple = false;
>
> + /* Pre-check candidate to skip the one which can not make a valid insn
> + during move_invariant_reg. */
> + if (flag_ira_loop_pressure && df_live && simple
> + && REG_P (dest) && DF_REG_DEF_COUNT (REGNO (dest)) > 1)
Why only do this with (flag_ira_loop_pressure && df_live)? If the
invariant can't be moved, we should ignore it regardless of whether
register pressure is taken into account.
> + {
> + df_ref use;
> + rtx ref;
> + unsigned int i = REGNO (dest);
> + struct df_insn_info *insn_info;
> + df_ref *def_rec;
> +
> + for (use = DF_REG_USE_CHAIN (i); use; use = DF_REF_NEXT_REG (use))
> + {
> + ref = DF_REF_INSN (use);
> + insn_info = DF_INSN_INFO_GET (ref);
> +
> + for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)
> + if (DF_REF_REGNO (*def_rec) == i)
> + {
> + /* Multi definitions at this stage, most likely are due to
> + instruction constrain, which requires both read and write
> + on the same register. Since move_invariant_reg is not
> + powerful enough to handle such cases, just ignore the INV
> + and leave the chance to others. */
> + return;
> + }
> + }
> + }
> +
> if (!may_assign_reg_p (SET_DEST (set))
> || !check_maybe_invariant (SET_SRC (set)))
> return;
Can you put your new check between "may_assign_reg_p (dest)" and
"check_maybe_invariant"? The may_assign_reg_p check is cheap and
triggers quite often.
Looks good to me otherwise.
Ciao!
Steven
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, loop2_invariant] Pre-check invariants
2014-06-10 11:01 ` Steven Bosscher
@ 2014-06-11 9:35 ` Zhenqiang Chen
2014-06-17 21:32 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Zhenqiang Chen @ 2014-06-11 9:35 UTC (permalink / raw)
To: Steven Bosscher; +Cc: gcc-patches
On 10 June 2014 19:01, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Tue, Jun 10, 2014 at 11:55 AM, Zhenqiang Chen wrote:
>>
>> * loop-invariant.c (find_invariant_insn): Skip invariants, which
>> can not make a valid insn during replacement in move_invariant_reg.
>>
>> --- a/gcc/loop-invariant.c
>> +++ b/gcc/loop-invariant.c
>> @@ -881,6 +881,35 @@ find_invariant_insn (rtx insn, bool
>> always_reached, bool always_executed)
>> || HARD_REGISTER_P (dest))
>> simple = false;
>>
>> + /* Pre-check candidate to skip the one which can not make a valid insn
>> + during move_invariant_reg. */
>> + if (flag_ira_loop_pressure && df_live && simple
>> + && REG_P (dest) && DF_REG_DEF_COUNT (REGNO (dest)) > 1)
>
> Why only do this with (flag_ira_loop_pressure && df_live)? If the
> invariant can't be moved, we should ignore it regardless of whether
> register pressure is taken into account.
Thanks for the comments. df_live seams redundant.
With flag_ira_loop_pressure, the pass will call df_analyze () at the
beginning, which can make sure all the DF info are correct.
Can we guarantee all DF_... correct without df_analyze ()?
>> + {
>> + df_ref use;
>> + rtx ref;
>> + unsigned int i = REGNO (dest);
>> + struct df_insn_info *insn_info;
>> + df_ref *def_rec;
>> +
>> + for (use = DF_REG_USE_CHAIN (i); use; use = DF_REF_NEXT_REG (use))
>> + {
>> + ref = DF_REF_INSN (use);
>> + insn_info = DF_INSN_INFO_GET (ref);
>> +
>> + for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)
>> + if (DF_REF_REGNO (*def_rec) == i)
>> + {
>> + /* Multi definitions at this stage, most likely are due to
>> + instruction constrain, which requires both read and write
>> + on the same register. Since move_invariant_reg is not
>> + powerful enough to handle such cases, just ignore the INV
>> + and leave the chance to others. */
>> + return;
>> + }
>> + }
>> + }
>> +
>> if (!may_assign_reg_p (SET_DEST (set))
>> || !check_maybe_invariant (SET_SRC (set)))
>> return;
>
>
> Can you put your new check between "may_assign_reg_p (dest)" and
> "check_maybe_invariant"? The may_assign_reg_p check is cheap and
> triggers quite often.
Updated and also removed the "flag_ira_loop_pressure && df_live". To
make it easy, I move the codes to a new function.
OK for trunk?
diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index c6bf19b..d19f3c8 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -839,6 +852,39 @@ check_dependencies (rtx insn, bitmap depends_on)
return true;
}
+/* Pre-check candidate DEST to skip the one which can not make a valid insn
+ during move_invariant_reg. SIMPlE is to skip HARD_REGISTER. */
+static bool
+pre_check_invariant_p (bool simple, rtx dest)
+{
+ if (simple && REG_P (dest) && DF_REG_DEF_COUNT (REGNO (dest)) > 1)
+ {
+ df_ref use;
+ rtx ref;
+ unsigned int i = REGNO (dest);
+ struct df_insn_info *insn_info;
+ df_ref *def_rec;
+
+ for (use = DF_REG_USE_CHAIN (i); use; use = DF_REF_NEXT_REG (use))
+ {
+ ref = DF_REF_INSN (use);
+ insn_info = DF_INSN_INFO_GET (ref);
+
+ for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)
+ if (DF_REF_REGNO (*def_rec) == i)
+ {
+ /* Multi definitions at this stage, most likely are due to
+ instruction constrain, which requires both read and write
+ on the same register. Since move_invariant_reg is not
+ powerful enough to handle such cases, just ignore the INV
+ and leave the chance to others. */
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
/* Finds invariant in INSN. ALWAYS_REACHED is true if the insn is always
executed. ALWAYS_EXECUTED is true if the insn is always executed,
unless the program ends due to a function call. */
@@ -868,7 +914,8 @@ find_invariant_insn (rtx insn, bool
always_reached, bool always_executed)
|| HARD_REGISTER_P (dest))
simple = false;
- if (!may_assign_reg_p (SET_DEST (set))
+ if (!may_assign_reg_p (dest)
+ || !pre_check_invariant_p (simple, dest)
|| !check_maybe_invariant (SET_SRC (set)))
return;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, loop2_invariant] Pre-check invariants
2014-06-11 9:35 ` Zhenqiang Chen
@ 2014-06-17 21:32 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2014-06-17 21:32 UTC (permalink / raw)
To: Zhenqiang Chen, Steven Bosscher; +Cc: gcc-patches
On 06/11/14 03:35, Zhenqiang Chen wrote:
>
> Thanks for the comments. df_live seams redundant.
>
> With flag_ira_loop_pressure, the pass will call df_analyze () at the
> beginning, which can make sure all the DF info are correct.
>
> Can we guarantee all DF_... correct without df_analyze ()?
They should be fine in this context.
> +/* Pre-check candidate DEST to skip the one which can not make a valid insn
> + during move_invariant_reg. SIMPlE is to skip HARD_REGISTER. */
s/SIMPlE/SIMPLE/
> + {
> + /* Multi definitions at this stage, most likely are due to
> + instruction constrain, which requires both read and write
s/constrain/constraints/
Though that doesn't make sense. Constraints don't come into play until
much later in the pipeline. Certainly there's been code in the
expanders and elsewhere to try and make the code we generate more
acceptable to 2-address targets and that's probably what you're really
running into. I think the code is fine, but that you need to improve
the comment.
ISTM that if your primary focus is to filter out read/write operands,
then just say that and ignore the constraints or other mechanisms by
which we got a read/write pseudo.
So I think with those two small comment changes, this patch is OK for
the trunk. Please post the final version for archival purposes before
checking it in.
Thanks,
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-17 21:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-10 9:55 [PATCH, loop2_invariant] Pre-check invariants Zhenqiang Chen
2014-06-10 11:01 ` Steven Bosscher
2014-06-11 9:35 ` Zhenqiang Chen
2014-06-17 21:32 ` Jeff Law
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).