re PR target/63783 ([SH] Miscompilation of boolean negation on SH4 using -O2)

gcc/
	PR target/63783
	PR target/51244
	* config/sh/sh_treg_combine.cc (sh_treg_combine::make_not_reg_insn):
	Do not emit bitwise not insn.  Emit logical not insn sequence instead.
	Adjust related comments throughout the file.

gcc/testsuite/
	PR target/63783
	PR target/51244
	* gcc.target/sh/torture/pr63783-1.c: New.
	* gcc.target/sh/torture/pr63783-2.c: New.
	* gcc.target/sh/pr51244-20.c: Adjust.
	* gcc.target/sh/pr51244-20-sh2a.c: Adjust.

From-SVN: r217969

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index bbab5022ffd5e4f45641503adf8c13b76a1232fc..cd28dc43474350f69a99527403831cafbb8b7cd4 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2014-11-22  Oleg Endo  <olegendo@gcc.gnu.org>
+
+       PR target/63783
+       PR target/51244
+       * config/sh/sh_treg_combine.cc (sh_treg_combine::make_not_reg_insn):
+       Do not emit bitwise not insn.  Emit logical not insn sequence instead.
+       Adjust related comments throughout the file.
+
 2014-11-22  Oleg Endo  <olegendo@gcc.gnu.org>
 
        PR target/63986

diff --git a/gcc/config/sh/sh_treg_combine.cc b/gcc/config/sh/sh_treg_combine.cc
index 4615a2843ffeaaec51c32fbd1e065db5b6f9e4a2..68c7fc1cc6d1c65c0d8e8bf2e327516ee24256c8 100644 (file)
--- a/gcc/config/sh/sh_treg_combine.cc
+++ b/gcc/config/sh/sh_treg_combine.cc
@@ -95,14 +95,17 @@ Example 1)
 
 In [bb 4] elimination of the comparison would require inversion of the branch
 condition and compensation of other BBs.
-Instead an inverting reg-move can be used:
+Instead the comparison in [bb 3] can be replaced with the comparison in [bb 5]
+by using a reg-reg move.  In [bb 4] a logical not is used to compensate the
+inverted condition.
 
 [bb 3]
 (set (reg:SI 167) (reg:SI 173))
 -> bb 5
 
 [BB 4]
-(set (reg:SI 167) (not:SI (reg:SI 177)))
+(set (reg:SI 147 t) (eq:SI (reg:SI 177) (const_int 0)))
+(set (reg:SI 167) (reg:SI 147 t))
 -> bb 5
 
 [bb 5]
@@ -231,9 +234,9 @@ In order to handle cases such as above the RTL pass does the following:
       and replace the comparisons in the BBs with reg-reg copies to get the
       operands in place (create new pseudo regs).
 
-    - If the cstores differ, try to apply the special case
-        (eq (reg) (const_int 0)) -> inverted = (not (reg)).
-      for the subordinate cstore types and eliminate the dominating ones.
+    - If the cstores differ and the comparison is a test against zero,
+      use reg-reg copies for the dominating cstores and logical not cstores
+      for the subordinate cstores.
 
 - If the comparison types in the BBs are not the same, or the first approach
   doesn't work out for some reason, try to eliminate the comparison before the
@@ -576,7 +579,8 @@ private:
   bool can_extend_ccreg_usage (const bb_entry& e,
                               const cbranch_trace& trace) const;
 
-  // Create an insn rtx that is a negating reg move (not operation).
+  // Create an insn rtx that performs a logical not (test != 0) on the src_reg
+  // and stores the result in dst_reg.
   rtx make_not_reg_insn (rtx dst_reg, rtx src_reg) const;
 
   // Create an insn rtx that inverts the ccreg.
@@ -907,12 +911,32 @@ sh_treg_combine::can_remove_comparison (const bb_entry& e,
 rtx
 sh_treg_combine::make_not_reg_insn (rtx dst_reg, rtx src_reg) const
 {
-  // This will to go through expanders and may output multiple insns
-  // for multi-word regs.
+  // On SH we can do only SImode and DImode comparisons.
+  if (! (GET_MODE (src_reg) == SImode || GET_MODE (src_reg) == DImode))
+    return NULL;
+
+  // On SH we can store the ccreg into an SImode or DImode reg only.
+  if (! (GET_MODE (dst_reg) == SImode || GET_MODE (dst_reg) == DImode))
+    return NULL;
+
   start_sequence ();
-  expand_simple_unop (GET_MODE (dst_reg), NOT, src_reg, dst_reg, 0);
+
+  emit_insn (gen_rtx_SET (VOIDmode, m_ccreg,
+                         gen_rtx_fmt_ee (EQ, SImode, src_reg, const0_rtx)));
+
+  if (GET_MODE (dst_reg) == SImode)
+    emit_move_insn (dst_reg, m_ccreg);
+  else if (GET_MODE (dst_reg) == DImode)
+    {
+      emit_move_insn (gen_lowpart (SImode, dst_reg), m_ccreg);
+      emit_move_insn (gen_highpart (SImode, dst_reg), const0_rtx);
+    }
+  else
+    gcc_unreachable ();
+
   rtx i = get_insns ();
   end_sequence ();
+
   return i;
 }
 
@@ -1095,7 +1119,12 @@ sh_treg_combine::try_combine_comparisons (cbranch_trace& trace,
   // There is one special case though, where an integer comparison
   //     (eq (reg) (const_int 0))
   // can be inverted with a sequence
-  //     (eq (not (reg)) (const_int 0))
+  //     (set (t) (eq (reg) (const_int 0))
+  //     (set (reg) (t))
+  //     (eq (reg) (const_int 0))
+  //
+  // FIXME: On SH2A it might be better to use the nott insn in this case,
+  // i.e. do the try_eliminate_cstores approach instead.
   if (inv_cstore_count != 0 && cstore_count != 0)
     {
       if (make_not_reg_insn (comp_op0, comp_op0) == NULL_RTX)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index e30e0ff5e14910afd35a73ba4298ead109197350..6cd4690dc9aefd29cc08c6765939db77c76ad776 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2014-11-22  Oleg Endo  <olegendo@gcc.gnu.org>
+
+       PR target/63783
+       PR target/51244
+       * gcc.target/sh/torture/pr63783-1.c: New.
+       * gcc.target/sh/torture/pr63783-2.c: New.
+       * gcc.target/sh/pr51244-20.c: Adjust.
+       * gcc.target/sh/pr51244-20-sh2a.c: Adjust.
+
 2014-11-22  Tobias Burnus  <burnus@net-b.de>
 
        * gfortran.dg/coarray/cosubscript_1.f90: New.

diff --git a/gcc/testsuite/gcc.target/sh/pr51244-20-sh2a.c b/gcc/testsuite/gcc.target/sh/pr51244-20-sh2a.c
index f2cd2de04f9b653b56bf608442a3ae780d4d60a2..3208f932addc13c684d3b179d9b24af080ffd609 100644 (file)
--- a/gcc/testsuite/gcc.target/sh/pr51244-20-sh2a.c
+++ b/gcc/testsuite/gcc.target/sh/pr51244-20-sh2a.c
@@ -3,12 +3,12 @@
 /* { dg-do compile }  */
 /* { dg-options "-O2" } */
 /* { dg-skip-if "" { "sh*-*-*" } { "*" } { "-m2a*" } } */
-/* { dg-final { scan-assembler-times "tst" 5 } } */
-/* { dg-final { scan-assembler-times "movt" 0 } } */
+/* { dg-final { scan-assembler-times "tst" 6 } } */
+/* { dg-final { scan-assembler-times "movt" 1 } } */
 /* { dg-final { scan-assembler-times "nott" 1 } } */
 /* { dg-final { scan-assembler-times "cmp/eq" 2 } } */
 /* { dg-final { scan-assembler-times "cmp/hi" 4 } } */
 /* { dg-final { scan-assembler-times "cmp/gt" 3 } } */
-/* { dg-final { scan-assembler-times "not\t" 1 } } */
+/* { dg-final { scan-assembler-not "not\t" } } */
 
 #include "pr51244-20.c"

diff --git a/gcc/testsuite/gcc.target/sh/pr51244-20.c b/gcc/testsuite/gcc.target/sh/pr51244-20.c
index a9ded46351114f1b916703d51b07357e9ab12667..aad6a2fd34fc67e511747729e6ba478313280a5c 100644 (file)
--- a/gcc/testsuite/gcc.target/sh/pr51244-20.c
+++ b/gcc/testsuite/gcc.target/sh/pr51244-20.c
@@ -1,15 +1,15 @@
 /* Check that the SH specific sh_treg_combine RTL optimization pass works as
    expected.  On SH2A the expected insns are slightly different, see
-   pr51244-21.c.  */
+   pr51244-20-sh2a.c.  */
 /* { dg-do compile }  */
 /* { dg-options "-O2" } */
 /* { dg-skip-if "" { "sh*-*-*" } { "-m5*" "-m2a*" } { "" } } */
-/* { dg-final { scan-assembler-times "tst" 6 } } */
-/* { dg-final { scan-assembler-times "movt" 1 } } */
+/* { dg-final { scan-assembler-times "tst" 7 } } */
+/* { dg-final { scan-assembler-times "movt" 2 } } */
 /* { dg-final { scan-assembler-times "cmp/eq" 2 } } */
 /* { dg-final { scan-assembler-times "cmp/hi" 4 } } */
 /* { dg-final { scan-assembler-times "cmp/gt" 2 } } */
-/* { dg-final { scan-assembler-times "not\t" 1 } } */
+/* { dg-final { scan-assembler-not "not\t" } } */
 
 
 /* non-SH2A: 2x tst, 1x movt, 2x cmp/eq, 1x cmp/hi
@@ -81,7 +81,7 @@ get_request_2 (int* q, int rw)
 }
 
 
-/* 2x tst, 1x cmp/hi, 1x not  */
+/* 3x tst, 1x movt, 1x cmp/hi, 1x not  */
 static inline int
 blk_oversized_queue_5 (int* q)
 {

diff --git a/gcc/testsuite/gcc.target/sh/torture/pr63783-1.c b/gcc/testsuite/gcc.target/sh/torture/pr63783-1.c
new file mode 100644 (file)
index 0000000..f18bead
--- /dev/null
+++ b/gcc/testsuite/gcc.target/sh/torture/pr63783-1.c
@@ -0,0 +1,29 @@
+/* { dg-do run }  */
+/* { dg-additional-options "-std=c99" }  */
+
+#include <assert.h>
+
+int decision_result;
+int val;
+int truecount = 0;
+
+static void __attribute__((noinline))
+buggy (int flag)
+{
+  int condition;
+  if(flag == 0)
+    condition = val != 0;
+  else
+    condition = !decision_result;
+  if (condition)
+     truecount++;
+}
+
+int
+main (void)
+{
+  decision_result = 1;
+  buggy(1);
+  assert (truecount == 0);
+  return 0;
+}

diff --git a/gcc/testsuite/gcc.target/sh/torture/pr63783-2.c b/gcc/testsuite/gcc.target/sh/torture/pr63783-2.c
new file mode 100644 (file)
index 0000000..c0bc911
--- /dev/null
+++ b/gcc/testsuite/gcc.target/sh/torture/pr63783-2.c
@@ -0,0 +1,29 @@
+/* { dg-do run }  */
+/* { dg-additional-options "-std=c99" }  */
+
+#include <assert.h>
+
+long long decision_result;
+long long val;
+int truecount = 0;
+
+static void __attribute__((noinline))
+buggy (int flag)
+{
+  int condition;
+  if(flag == 0)
+    condition = val != 0;
+  else
+    condition = !decision_result;
+  if (condition)
+     truecount++;
+}
+
+int
+main (void)
+{
+  decision_result = 1;
+  buggy(1);
+  assert (truecount == 0);
+  return 0;
+}