[4.8 & 4.9] Backport of r211885

Discussion:

Felix Yang

2014-10-08 15:00:24 UTC

The enclosed patch for 4.8 & 4.9 branch is a backport of r211885 from trunk.

The only change is to use:

for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)

other than the new FOR_EACH_INSN_INFO_DEF interface.

Bootstrapped on x86_64-SUSE-Linux for both branches. OK to apply?

Index: gcc/loop-invariant.c
===================================================================
--- gcc/loop-invariant.c (revision 216001)
+++ gcc/loop-invariant.c (working copy)
@@ -839,6 +839,41 @@ 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 constraints, 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. */
@@ -869,6 +904,7 @@ find_invariant_insn (rtx insn, bool always_reached
simple = false;

if (!may_assign_reg_p (SET_DEST (set))
+ || !pre_check_invariant_p (simple, dest)
|| !check_maybe_invariant (SET_SRC (set)))
return;

Cheers,
Felix

Yangfei (Felix)

2014-10-09 08:50:07 UTC

Permalink

PING?

Post by Felix Yang
The enclosed patch for 4.8 & 4.9 branch is a backport of r211885 from trunk.
for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++)
other than the new FOR_EACH_INSN_INFO_DEF interface.
Bootstrapped on x86_64-SUSE-Linux for both branches. OK to apply?
Index: gcc/loop-invariant.c
================================================================
===
--- gcc/loop-invariant.c (revision 216001)
+++ gcc/loop-invariant.c (working copy)
@@ -839,6 +839,41 @@ 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 constraints, 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,
@@ find_invariant_insn (rtx insn, bool always_reached
simple = false;
if (!may_assign_reg_p (SET_DEST (set))
+ || !pre_check_invariant_p (simple, dest)
|| !check_maybe_invariant (SET_SRC (set)))
return;

Jakub Jelinek

2014-10-09 08:53:53 UTC

Permalink

ChangeLog entry is missing, plus description why do you want to backport it.
If it fixes a bug on the branches, it would be better to have a bugzilla
PR for that, and definitely a testcase.

Post by Felix Yang
--- gcc/loop-invariant.c (revision 216001)
+++ gcc/loop-invariant.c (working copy)
@@ -839,6 +839,41 @@ 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 constraints, 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. */
@@ -869,6 +904,7 @@ find_invariant_insn (rtx insn, bool always_reached
simple = false;
if (!may_assign_reg_p (SET_DEST (set))
+ || !pre_check_invariant_p (simple, dest)
|| !check_maybe_invariant (SET_SRC (set)))
return;
Cheers,
Felix

Jakub

Yangfei (Felix)

2014-10-09 09:04:49 UTC

Permalink

Post by Jakub Jelinek

ChangeLog entry is missing, plus description why do you want to backport it.
If it fixes a bug on the branches, it would be better to have a bugzilla PR for that,
and definitely a testcase.

Yeah, I will add a ChangeLog entry for this patch when it is committed.
I encountered the same issue when working on my local customized 4.8/4.9 branches. Not reproduceable with the official 4.8/4.9 branches.
I thinks it's just an enhancement for the loop invariant pass to make it more versatile. It's better that 4.8/4.9 branches

Jakub Jelinek

2014-10-09 09:07:29 UTC