On Fri, Apr 08, 2016 at 01:18:50PM +0200, Bernd Schmidt wrote:
> On 04/07/2016 11:56 PM, Jakub Jelinek wrote:
> >Not sure if this patch catches everything though, perhaps there could be
> >e.g.
> >(set (reg:SI ...) (plus:SI ((subreg:SI (reg:QI ...) 0) (const_int ...)))
> >and we'd still assign REG_EQUAL note. So maybe instead we should walk the
> >*loc expression and look for paradoxical subregs, and for each of them, if
> >we find the DF_REF_REG (use) mentioned in their operand, clear
> >set_reg_equal. Though of course, if DF_REF_REG (use) itself is a
> >paradoxical subreg, we could clear set_reg_equal without any walking.
>
> It seems like something like that could happen.
>
> How much do we lose if we just don't make new REG_EQUAL notes here?
After IRC discussions, I've bootstrapped/regtested following patch that
just punts if *loc contains any paradoxical subregs, together with
additional statistics gathering that proved that the new testcase is
the only spot in which this patch makes a difference on x86_64-linux
and i686-linux bootstrap/regtest.
Of course on other targets it might affect more.
Ok for trunk?
2016-04-08 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/70574
* fwprop.c (forward_propagate_and_simplify): Don't add
REG_EQUAL note if DF_REF_REG (use) is a paradoxical subreg.
(try_fwprop_subst): Don't add REG_EQUAL note if there are any
paradoxical subregs within *loc.
* gcc.target/i386/avx2-pr70574.c: New test.
--- gcc/fwprop.c.jj 2016-04-07 23:27:41.396310248 +0200
+++ gcc/fwprop.c 2016-04-08 15:51:09.164706722 +0200
@@ -999,10 +999,27 @@ try_fwprop_subst (df_ref use, rtx *loc,
making a new one if one does not already exist. */
if (set_reg_equal)
{
- if (dump_file)
- fprintf (dump_file, " Setting REG_EQUAL note\n");
+ /* If there are any paradoxical SUBREGs, don't add REG_EQUAL note,
+ because the bits in there can be anything and so might not
+ match the REG_EQUAL note content. See PR70574. */
+ subrtx_var_iterator::array_type array;
+ FOR_EACH_SUBRTX_VAR (iter, array, *loc, NONCONST)
+ {
+ rtx x = *iter;
+ if (SUBREG_P (x) && paradoxical_subreg_p (x))
+ {
+ set_reg_equal = false;
+ break;
+ }
+ }
+
+ if (set_reg_equal)
+ {
+ if (dump_file)
+ fprintf (dump_file, " Setting REG_EQUAL note\n");
- note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (new_rtx));
+ note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (new_rtx));
+ }
}
}
@@ -1300,14 +1317,19 @@ forward_propagate_and_simplify (df_ref u
that isn't mentioned in USE_SET, as the note would be invalid
otherwise. We also don't want to install a note if we are merely
propagating a pseudo since verifying that this pseudo isn't dead
- is a pain; moreover such a note won't help anything. */
+ is a pain; moreover such a note won't help anything.
+ If the use is a paradoxical subreg, make sure we don't add a
+ REG_EQUAL note for it, because it is not equivalent, it is one
+ possible value for it, but we can't rely on it holding that value.
+ See PR70574. */
set_reg_equal = (note == NULL_RTX
&& REG_P (SET_DEST (use_set))
&& !REG_P (src)
&& !(GET_CODE (src) == SUBREG
&& REG_P (SUBREG_REG (src)))
&& !reg_mentioned_p (SET_DEST (use_set),
- SET_SRC (use_set)));
+ SET_SRC (use_set))
+ && !paradoxical_subreg_p (DF_REF_REG (use)));
}
if (GET_MODE (*loc) == VOIDmode)
--- gcc/testsuite/gcc.target/i386/avx2-pr70574.c.jj 2016-04-08
13:32:04.525196849 +0200
+++ gcc/testsuite/gcc.target/i386/avx2-pr70574.c 2016-04-08
13:32:04.525196849 +0200
@@ -0,0 +1,26 @@
+/* PR rtl-optimization/70574 */
+/* { dg-do run { target lp64 } } */
+/* { dg-require-effective-target avx2 } */
+/* { dg-options "-O -frerun-cse-after-loop -fno-tree-ccp -mcmodel=medium
-mavx2" } */
+/* { dg-additional-options "-fPIC" { target fpic } } */
+
+#include "avx2-check.h"
+
+typedef char A __attribute__((vector_size (32)));
+typedef short B __attribute__((vector_size (32)));
+
+int
+foo (int x, __int128 y, __int128 z, A w)
+{
+ y <<= 64;
+ w *= (A) { 0, -1, z, 0, ~y };
+ return w[0] + ((B) { x, 0, y, 0, -1 } | 1)[4];
+}
+
+static void
+avx2_test ()
+{
+ int x = foo (0, 0, 0, (A) {});
+ if (x != -1)
+ __builtin_abort ();
+}
Jakub
