Hi!

As mentioned in the PR, if we have
simplify_shift_const_1 (code=LSHIFTRT, result_mode=SImode, 
varop=0x7ffff184de70, orig_count=31)
where varop is:
(subreg:SI (lshiftrt:DI (const_int -1 [0xffffffffffffffff])
        (subreg:QI (reg:SI 100) 0)) 0),
we optimize the inner shift (mode == DImode) into
(lshiftrt:DI (const_int 8589934591 [0x1ffffffff])
        (subreg:QI (reg:SI 100) 0)) 0)
but because result_mode == shift_mode == SImode (only mode == DImode),
we don't perform the needed masking, the outer shift is with new
count == 0 and thus not done at all.
Seems testsuite coverage for this function is very low and I'm sure there
is a lot of other bugs in that function; I've performed bootstrap/regtest
on x86_64-linux and i686-linux with this patch (without the && orig_count !=
0 && outer_op == UNKNOWN && !complement_p) part, with additional gathering
of interesting calls to this function, see the PR for some details,
but the summary is that this patch triggers only on the new testcases and
nothing else during bootstrap/regtest, fixes the testcase and while in
theory it could pessimize some cases (either by adding some obviously
correct, but unneeded AND around it, or by combiner not combining
something), it shouldn't break stuff (which is why I've added the
extra checks, if there are outer ops (outer_op or complement_p), the
patch could change the generated code and break something).
For GCC 7, I believe we should try to add sufficient testsuite coverage for
all the different cases (e.g. for all the cases where it sets count == 0,
in each case with mode != result_mode and mode == result_mode).

Ok for trunk?

2016-03-15  Jakub Jelinek  <ja...@redhat.com>

        PR rtl-optimization/70222
        * combine.c (simplify_shift_const_1): Force simplify_and_const_int
        for LSHIFTRT if result_mode != mode && count == 0 and no outer
        operation is scheduled.

        * gcc.c-torture/execute/pr70222-1.c: New test.
        * gcc.c-torture/execute/pr70222-2.c: New test.

--- gcc/combine.c.jj    2016-03-14 23:18:37.958408627 +0100
+++ gcc/combine.c       2016-03-15 11:47:24.474968676 +0100
@@ -10835,8 +10835,16 @@ simplify_shift_const_1 (enum rtx_code co
     x = simplify_gen_binary (code, shift_mode, varop, GEN_INT (count));
 
   /* If we were doing an LSHIFTRT in a wider mode than it was originally,
-     turn off all the bits that the shift would have turned off.  */
-  if (orig_code == LSHIFTRT && result_mode != shift_mode)
+     turn off all the bits that the shift would have turned off.
+     Similarly do this if we've optimized varop so that we don't perform
+     any shift.  */
+  if (orig_code == LSHIFTRT
+      && (result_mode != shift_mode
+         || (result_mode != mode
+             && count == 0
+             && orig_count != 0
+             && outer_op == UNKNOWN
+             && !complement_p)))
     x = simplify_and_const_int (NULL_RTX, shift_mode, x,
                                GET_MODE_MASK (result_mode) >> orig_count);
 
--- gcc/testsuite/gcc.c-torture/execute/pr70222-1.c.jj  2016-03-15 
11:30:41.657000384 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr70222-1.c     2016-03-15 
11:30:41.657000384 +0100
@@ -0,0 +1,30 @@
+/* PR rtl-optimization/70222 */
+
+int a = 1;
+unsigned int b = 2;
+int c = 0;
+int d = 0;
+
+void
+foo ()
+{
+  int e = ((-(c >= c)) < b) > ((int) (-1ULL >> ((a / a) * 15)));
+  d = -e;
+}
+
+__attribute__((noinline, noclone)) void
+bar (int x)
+{
+  if (x != -1)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+  foo ();
+  bar (d);
+#endif
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr70222-2.c.jj  2016-03-15 
11:36:13.273366841 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr70222-2.c     2016-03-15 
11:36:18.156298614 +0100
@@ -0,0 +1,20 @@
+/* PR rtl-optimization/70222 */
+
+#if __CHAR_BIT__ == 8 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+__attribute__((noinline, noclone)) unsigned int
+foo (int x)
+{
+  unsigned long long y = -1ULL >> x;
+  return (unsigned int) y >> 31;
+}
+#endif
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+  if (foo (15) != 1 || foo (32) != 1 || foo (33) != 0)
+    __builtin_abort ();
+#endif
+  return 0;
+}

        Jakub

Reply via email to