Richard Sandiford <richard.sandif...@arm.com> writes:
> Jakub Jelinek <ja...@redhat.com> writes:
>> On Mon, Mar 03, 2025 at 01:46:20PM +0000, Richard Sandiford wrote:
>>> Jakub Jelinek <ja...@redhat.com> writes:
>>> > On Mon, Mar 03, 2025 at 01:02:07PM +0000, Richard Sandiford wrote:
>>> >> ...how about something like this?  Completely untested, and I haven't
>>> >> thought about it much.  Just didn't want to hold up the discussion.
>>> >
>>> > Works for me.
>>> >
>>> > Just wonder if there is anything that will actually verify that XEXP 
>>> > (op0, 0)
>>> > and XEXP (op1, 0) modes are at least from the same class, rather than say
>>> > have one of the comparisons in MODE_CC and another in MODE_INT or vice 
>>> > versa
>>> > or whatever other modes.
>>> 
>>> There's:
>>> 
>>>   if (!(rtx_equal_p (XEXP (op0, 0), XEXP (op1, 0))
>>>     && rtx_equal_p (XEXP (op0, 1), XEXP (op1, 1))))
>>>     return 0;
>>
>> You're right, and rtx_equal_p returns false for GET_MODE differences.
>>
>> Will you test your patch (with the testcase from my patch) or should I?
>
> I'm just about to test it with a tweak to the mode check.  Should be
> done in a couple of hours.

Heh.  Forgot when I said that that I would need to test on powerpc64le
as well, which isn't something I do regularly, and so took a couple of
gos to get right.  I've now bootstrapped & regression-tested on both
powerpc64le-linux-gnu and aarch64-linux-gnu.  OK to install?

Richard


The following testcase is miscompiled on powerpc64le-linux starting with
r15-6777.  During combine we see:

(set (reg:SI 134)
    (ior:SI (ge:SI (reg:CCFP 128)
            (const_int 0 [0]))
        (lt:SI (reg:CCFP 128)
            (const_int 0 [0]))))

The simplify_logical_relational_operation code (in its current form)
was written with arithmetic rather than CC modes in mind.  Since CCFP
is a CC mode, it fails the HONOR_NANS check, and so the function assumes
that ge | lt => true.

If one comparison is unsigned then it should be safe to assume that
the other comparison is also unsigned, even for CC modes, since the
optimisation checks that the comparisons are between the same operands.
For the other cases, we can only safely fold comparisons of CC mode
values if the result is always-true (15) or always-false (0).

It turns out that the original testcase for PR117186, which ran at -O,
was relying on the old behaviour for some of the functions.  It needs
4-instruction combinations, and so -fexpensive-optimizations, to pass
in its intended form.

gcc/
        PR rtl-optimization/119002
        * simplify-rtx.cc
        (simplify_context::simplify_logical_relational_operation): Handle
        comparisons between CC values.  If there is no evidence that the
        CC values are unsigned, restrict the fold to always-true or
        always-false results.

gcc/testsuite/
        * gcc.c-torture/execute/ieee/pr119002.c: New test.
        * gcc.target/aarch64/pr117186.c: Run at -O2 rather than -O.

Co-authored-by: Jakub Jelinek <ja...@redhat.com>
---
 gcc/simplify-rtx.cc                           | 12 ++++++++--
 .../gcc.c-torture/execute/ieee/pr119002.c     | 23 +++++++++++++++++++
 gcc/testsuite/gcc.target/aarch64/pr117186.c   |  2 +-
 3 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index c478bd060fc..fe007bc7d96 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -2655,6 +2655,9 @@ simplify_context::simplify_logical_relational_operation 
(rtx_code code,
 
   enum rtx_code code0 = GET_CODE (op0);
   enum rtx_code code1 = GET_CODE (op1);
+  machine_mode cmp_mode = GET_MODE (XEXP (op0, 0));
+  if (cmp_mode == VOIDmode)
+    cmp_mode = GET_MODE (XEXP (op0, 1));
 
   /* Assume at first that the comparisons are on integers, and that the
      operands are therefore ordered.  */
@@ -2672,8 +2675,10 @@ simplify_context::simplify_logical_relational_operation 
(rtx_code code,
     }
   else
     {
-      /* See whether the operands might be unordered.  */
-      if (HONOR_NANS (GET_MODE (XEXP (op0, 0))))
+      /* See whether the operands might be unordered.  Assume that all
+        results are possible for CC modes, and punt later if we don't get an
+        always-true or always-false answer.  */
+      if (GET_MODE_CLASS (cmp_mode) == MODE_CC || HONOR_NANS (cmp_mode))
        all = 15;
       mask0 = comparison_to_mask (code0) & all;
       mask1 = comparison_to_mask (code1) & all;
@@ -2702,6 +2707,9 @@ simplify_context::simplify_logical_relational_operation 
(rtx_code code,
     code = mask_to_unsigned_comparison (mask);
   else
     {
+      if (GET_MODE_CLASS (cmp_mode) == MODE_CC)
+       return 0;
+
       code = mask_to_comparison (mask);
       /* LTGT and NE are arithmetically equivalent for ordered operands,
         with NE being the canonical choice.  */
diff --git a/gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c 
b/gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c
new file mode 100644
index 00000000000..af1a705f170
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c
@@ -0,0 +1,23 @@
+/* PR rtl-optimization/119002 */
+
+__attribute__((noipa)) unsigned int
+foo (void *x, float y, float z)
+{
+  unsigned int a, b;
+  float c, d, e;
+  c = y;
+  d = z;
+  a = c < d;
+  d = y;
+  e = z;
+  b = d >= e;
+  a |= b;
+  return a;
+}
+
+int
+main ()
+{
+  if (foo ((void *) 0, 0.f, __builtin_nanf ("")))
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr117186.c 
b/gcc/testsuite/gcc.target/aarch64/pr117186.c
index afe3c25a493..3090f2c11d5 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr117186.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr117186.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O" } */
+/* { dg-options "-O2" } */
 /* { dg-final { check-function-bodies "**" "" "" } } */
 
 /*
-- 
2.25.1

Reply via email to