On 10 June 2014 19:01, Steven Bosscher <stevenb....@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;

Reply via email to