diff mbox series

Fix bad interaction between ce and cmpelim passes on Visium

Message ID 1780456.IWWpvtJA7H@polaris
State New
Headers show
Series Fix bad interaction between ce and cmpelim passes on Visium | expand

Commit Message

Eric Botcazou Nov. 19, 2018, 10:27 a.m. UTC
This is a code quality regression that appeared during GCC 8 development on 
Visium in the form of the failure of gcc.target/visium/overflow*.c tests and 
that I swept under the rug back then.

On architectures which do not provide scc instructions like Visium, you can 
nevertheless implement a few variants of cstore by using the carry, which is 
traditionally modeled as sltu, and some tricks (for example, on Visium, we 
implement seq/sne/sltu/sgtu/sleu/sgeu this way); the only counterpart is that 
the patterns are a bit convoluted if you want to have exact RTL semantics.

So generating cstore instructions, usually done by the if-conversion pass, can 
disable the elimination of redundant comparison instructions, only done by the 
compare-elim pass on some RISC architectures, because the latter pass is based 
on generic pattern matching.  That's what happened here for UNEGV operations.

The regression is fixed by teaching the compare-elim pass to handle a NOT in 
the first operand of comparisons and adjusting the neg<mode>2_insn_set_carry 
of the Visium to make it consistent with the unegv<mode>3 pattern.

Tested on visium-elf, sparc-sun-solaris2.11 and x86_64-suse-linux, applied on 
the mainline.


2018-11-19  Eric Botcazou  <ebotcazou@adacore.com>

	* compare-elim.c (struct comparison): Add not_in_a field.
	(is_not): New static function.
	(strip_not): Likewise.
	(conforming_compare): Handle a NOT in the first operand.
	(can_eliminate_compare): Likewise.
	(find_comparison_dom_walker::before_dom_children): Likewise.
	(try_eliminate_compare): Likewise.
	* config/visium/visium.md (negsi2_insn_set_carry): Turn into...
	(neg<mode>2_insn_set_carry): ...this and add missing NEG operation.


2018-11-19  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.target/visium/overflow8.c: Remove -fno-if-conversion and
	unrelated final test.
	* gcc.target/visium/overflow16: Likewise.
	* gcc.target/visium/overflow32.c: Likewise.
diff mbox series

Patch

Index: compare-elim.c
===================================================================
--- compare-elim.c	(revision 266178)
+++ compare-elim.c	(working copy)
@@ -123,10 +123,32 @@  struct comparison
 
   /* True if its inputs are still valid at the end of the block.  */
   bool inputs_valid;
+
+  /* Whether IN_A is wrapped in a NOT before being compared.  */
+  bool not_in_a;
 };
   
 static vec<comparison *> all_compares;
 
+/* Return whether X is a NOT unary expression.  */
+
+static bool
+is_not (rtx x)
+{
+  return GET_CODE (x) == NOT;
+}
+
+/* Strip a NOT unary expression around X, if any.  */
+
+static rtx
+strip_not (rtx x)
+{
+  if (is_not (x))
+    return XEXP (x, 0);
+
+  return x;
+}
+
 /* Look for a "conforming" comparison, as defined above.  If valid, return
    the rtx for the COMPARE itself.  */
 
@@ -147,7 +169,7 @@  conforming_compare (rtx_insn *insn)
   if (!REG_P (dest) || REGNO (dest) != targetm.flags_regnum)
     return NULL;
 
-  if (!REG_P (XEXP (src, 0)))
+  if (!REG_P (strip_not (XEXP (src, 0))))
     return NULL;
 
   if (CONSTANT_P (XEXP (src, 1)) || REG_P (XEXP (src, 1)))
@@ -278,10 +300,13 @@  can_eliminate_compare (rtx compare, rtx
     return false;
 
   /* Make sure the compare is redundant with the previous.  */
-  if (!rtx_equal_p (XEXP (compare, 0), cmp->in_a)
+  if (!rtx_equal_p (strip_not (XEXP (compare, 0)), cmp->in_a)
       || !rtx_equal_p (XEXP (compare, 1), cmp->in_b))
     return false;
 
+  if (is_not (XEXP (compare, 0)) != cmp->not_in_a)
+    return false;
+
   /* New mode must be compatible with the previous compare mode.  */
   machine_mode new_mode
     = targetm.cc_modes_compatible (GET_MODE (compare), cmp->orig_mode);
@@ -365,8 +390,9 @@  find_comparison_dom_walker::before_dom_c
 	  last_cmp = XCNEW (struct comparison);
 	  last_cmp->insn = insn;
 	  last_cmp->prev_clobber = last_clobber;
-	  last_cmp->in_a = XEXP (src, 0);
+	  last_cmp->in_a = strip_not (XEXP (src, 0));
 	  last_cmp->in_b = XEXP (src, 1);
+	  last_cmp->not_in_a = is_not (XEXP (src, 0));
 	  last_cmp->eh_note = eh_note;
 	  last_cmp->orig_mode = GET_MODE (src);
 	  if (last_cmp->in_b == const0_rtx
@@ -837,7 +863,9 @@  try_eliminate_compare (struct comparison
     flags = gen_rtx_REG (cmp->orig_mode, targetm.flags_regnum);
 
   /* Generate a new comparison for installation in the setter.  */
-  rtx y = copy_rtx (cmp_a);
+  rtx y = cmp->not_in_a
+	  ? gen_rtx_NOT (GET_MODE (cmp_a), copy_rtx (cmp_a))
+	  : copy_rtx (cmp_a);
   y = gen_rtx_COMPARE (GET_MODE (flags), y, copy_rtx (cmp_b));
   y = gen_rtx_SET (flags, y);
 
Index: config/visium/visium.md
===================================================================
--- config/visium/visium.md	(revision 266178)
+++ config/visium/visium.md	(working copy)
@@ -1208,14 +1208,14 @@  (define_insn "*neg<mode>2_insn<subst_ari
   "sub<s>   %0,r0,%1"
   [(set_attr "type" "arith")])
 
-(define_insn "negsi2_insn_set_carry"
+(define_insn "neg<mode>2_insn_set_carry"
   [(set (reg:CCC R_FLAGS)
-	(compare:CCC (not:SI (match_operand:SI 1 "register_operand" "r"))
+	(compare:CCC (not:I (neg:I (match_operand:I 1 "register_operand" "r")))
 		     (const_int -1)))
-   (set (match_operand:SI 0 "register_operand" "=r")
-        (neg:SI (match_dup 1)))]
+   (set (match_operand:I 0 "register_operand" "=r")
+        (neg:I (match_dup 1)))]
   "reload_completed"
-  "sub.l   %0,r0,%1"
+  "sub<s>   %0,r0,%1"
   [(set_attr "type" "arith")])
 
 (define_insn "*neg<mode>2_insn_set_overflow"
Index: testsuite/gcc.target/visium/overflow16.c
===================================================================
--- testsuite/gcc.target/visium/overflow16.c	(revision 266178)
+++ testsuite/gcc.target/visium/overflow16.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -fno-if-conversion" } */
+/* { dg-options "-O2" } */
 
 #include <stdbool.h>
 
@@ -36,4 +36,3 @@  bool my_neg_overflow (short a, short *re
 /* { dg-final { scan-assembler-times "add.w" 2 } } */
 /* { dg-final { scan-assembler-times "sub.w" 4 } } */
 /* { dg-final { scan-assembler-not "cmp.w" } } */
-/* { dg-final { scan-assembler-not "mov.w" } } */
Index: testsuite/gcc.target/visium/overflow32.c
===================================================================
--- testsuite/gcc.target/visium/overflow32.c	(revision 266178)
+++ testsuite/gcc.target/visium/overflow32.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -fno-if-conversion" } */
+/* { dg-options "-O2" } */
 
 #include <stdbool.h>
 
@@ -36,4 +36,3 @@  bool my_neg_overflow (int a, int *res)
 /* { dg-final { scan-assembler-times "add.l" 2 } } */
 /* { dg-final { scan-assembler-times "sub.l" 4 } } */
 /* { dg-final { scan-assembler-not "cmp.l" } } */
-/* { dg-final { scan-assembler-not "mov.l" } } */
Index: testsuite/gcc.target/visium/overflow8.c
===================================================================
--- testsuite/gcc.target/visium/overflow8.c	(revision 266178)
+++ testsuite/gcc.target/visium/overflow8.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -fno-if-conversion" } */
+/* { dg-options "-O2" } */
 
 #include <stdbool.h>
 
@@ -36,4 +36,3 @@  bool my_neg_overflow (signed char a, sig
 /* { dg-final { scan-assembler-times "add.b" 2 } } */
 /* { dg-final { scan-assembler-times "sub.b" 4 } } */
 /* { dg-final { scan-assembler-not "cmp.b" } } */
-/* { dg-final { scan-assembler-not "mov.b" } } */