As outlined in the BZ simplify_logical_relational_operation does not validate
that the comparison code it selects is ultimately valid for the mode of the
comparison.

So we could have something like:

(ior (lt (...)) (gt (...))

Which it happily turns into

(ltgt (...))

Which is not valid for integral modes.

This patch validates the selected comparison code is valid for the comparison's
mode using the rules in rtl.def.  It's always safe to reject a particular
simplification.

I've also verified this fixes 94144 which is a similar problem showing up on
aarch64 and that it doesn't cause regressions in the tester.

I'm committing this to the trunk.

Jeff
commit fd25ae64e3ffaa95abb60e74af45031d2f5f900f
Author: Jeff Law <l...@redhat.com>
Date:   Mon Mar 23 17:55:20 2020 -0600

    Verify the code used for the optimized comparison is
    valid for the comparison's mode.
    
            PR rtl-optimization/90275
            PR target/94238
            PR target/94144
            * simplify-rtx.c (comparison_code_valid_for_mode): New function.
            (simplify_logical_relational_operation): Use it.
    
            PR target/94144
            PR target/94238
            * gcc.c-torture/compile/pr94144.c: New test.
            * gcc.c-torture/compile/pr94238.c: New test.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9ba58273c21..3b926703957 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-03-23  Jeff Law  <l...@redhat.com>
+
+       PR rtl-optimization/90275
+       PR target/94238
+       PR target/94144
+       * simplify-rtx.c (comparison_code_valid_for_mode): New function.
+       (simplify_logical_relational_operation): Use it.
+
 2020-03-23  Jakub Jelinek  <ja...@redhat.com>
 
        PR c++/91993
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index dd3d85156c3..28c2dc69ae7 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -2215,6 +2215,53 @@ mask_to_comparison (int mask)
     }
 }
 
+/* Return true if CODE is valid for comparisons of mode MODE, false
+   otherwise.
+
+   It is always safe to return false, even if the code was valid for the
+   given mode as that will merely suppress optimizations.  */
+
+static bool
+comparison_code_valid_for_mode (enum rtx_code code, enum machine_mode mode)
+{
+  switch (code)
+    {
+      /* These are valid for integral, floating and vector modes.  */
+      case NE:
+      case EQ:
+      case GE:
+      case GT:
+      case LE:
+      case LT:
+       return (INTEGRAL_MODE_P (mode)
+               || FLOAT_MODE_P (mode)
+               || VECTOR_MODE_P (mode));
+
+      /* These are valid for floating point modes.  */
+      case LTGT:
+      case UNORDERED:
+      case ORDERED:
+      case UNEQ:
+      case UNGE:
+      case UNGT:
+      case UNLE:
+      case UNLT:
+       return FLOAT_MODE_P (mode);
+
+      /* These are filtered out in simplify_logical_operation, but
+        we check for them too as a matter of safety.   They are valid
+        for integral and vector modes.  */
+      case GEU:
+      case GTU:
+      case LEU:
+      case LTU:
+       return INTEGRAL_MODE_P (mode) || VECTOR_MODE_P (mode);
+
+      default:
+       gcc_unreachable ();
+    }
+}
+                                      
 /* Simplify a logical operation CODE with result mode MODE, operating on OP0
    and OP1, which should be both relational operations.  Return 0 if no such
    simplification is possible.  */
@@ -2252,6 +2299,10 @@ simplify_logical_relational_operation (enum rtx_code 
code, machine_mode mode,
 
   code = mask_to_comparison (mask);
 
+  /* Many comparison codes are only valid for certain mode classes.  */
+  if (!comparison_code_valid_for_mode (code, mode))
+    return 0;
+
   op0 = XEXP (op1, 0);
   op1 = XEXP (op1, 1);
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5f079f1fca9..28adfd8b580 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2020-03-23  Jeff Law  <l...@redhat.com>
+
+       PR target/94144
+       PR target/94238
+       * gcc.c-torture/compile/pr94144.c: New test.
+       * gcc.c-torture/compile/pr94238.c: New test.
+       
 2020-03-23  Patrick Palka  <ppa...@redhat.com>
 
        PR c++/93805
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr94144.c 
b/gcc/testsuite/gcc.c-torture/compile/pr94144.c
new file mode 100644
index 00000000000..4358e0a7b00
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr94144.c
@@ -0,0 +1,18 @@
+
+int a, b, z;
+int c(int d, int e) { return d && e > 0 && d > 5 - e ? 0 : d + e; }
+int k();
+void h(int);
+void f(short d) {
+  int g = !(0 < d);
+  h(d);
+  if (b) {
+    unsigned i[1];
+    i[0] = g = 0;
+    for (; g <= 8; g++)
+      d || k();
+    if (c(!(i[0] <= z) >= d, d) != a)
+      k();
+  }
+}
+
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr94238.c 
b/gcc/testsuite/gcc.c-torture/compile/pr94238.c
new file mode 100644
index 00000000000..5a96a64d85b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr94238.c
@@ -0,0 +1,22 @@
+enum { false, true } a;
+int b, c, d, e, f;
+int fn3();
+void fn2();
+
+void fn1() {
+  _Bool g, h = false, i = false;
+  int j;
+  c = b && f || d;
+  if (c) {
+    if (d)
+      i = true;
+    _Bool k = b;
+    int l = e, m = a;
+    g = k && l < m || l > m;
+  }
+  if (g)
+    h = true;
+  if (i)
+    fn2();
+  h &&j &&fn3();
+}

Reply via email to