From 10f916167ccf8451fb717689eef61d3895c1f2a2 Mon Sep 17 00:00:00 2001 From: Oleg Endo Date: Sat, 22 Nov 2014 15:50:10 +0000 Subject: [PATCH] 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 --- gcc/ChangeLog | 8 +++ gcc/config/sh/sh_treg_combine.cc | 49 +++++++++++++++---- gcc/testsuite/ChangeLog | 9 ++++ gcc/testsuite/gcc.target/sh/pr51244-20-sh2a.c | 6 +-- gcc/testsuite/gcc.target/sh/pr51244-20.c | 10 ++-- .../gcc.target/sh/torture/pr63783-1.c | 29 +++++++++++ .../gcc.target/sh/torture/pr63783-2.c | 29 +++++++++++ 7 files changed, 122 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/gcc.target/sh/torture/pr63783-1.c create mode 100644 gcc/testsuite/gcc.target/sh/torture/pr63783-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index bbab5022ffd..cd28dc43474 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2014-11-22 Oleg Endo + + 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 PR target/63986 diff --git a/gcc/config/sh/sh_treg_combine.cc b/gcc/config/sh/sh_treg_combine.cc index 4615a2843ff..68c7fc1cc6d 100644 --- 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 e30e0ff5e14..6cd4690dc9a 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2014-11-22 Oleg Endo + + 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 * 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 f2cd2de04f9..3208f932add 100644 --- 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 a9ded463511..aad6a2fd34f 100644 --- 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 index 00000000000..f18beadd9c8 --- /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 + +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 index 00000000000..c0bc9116c23 --- /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 + +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; +} -- 2.30.2