[Bug tree-optimization/107046] New: [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[Bug tree-optimization/107046] New: [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[jeffreyalaw at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

            Bug ID: 107046
           Summary: [13 Regression] Recent FP range work causing inf-2 to
                    be miscompiled on rx-elf
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jeffreyalaw at gmail dot com
  Target Milestone: ---

gcc.c-torture/execute/ieee/inf-2.c is being mis-compiled as of:

commit b7fd7fb5011106c062df9275ca8fddcbce4ebdeb
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Sep 22 18:20:39 2022 +0200

    frange: drop endpoints to min/max representable numbers for
-ffinite-math-only.

    Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
    think we can drop the numbers outside of the min/max representable
    numbers to the representable number.

    This means the endpoings to VR_VARYING for -ffinite-math-only can now
    be the min/max representable, instead of -INF and +INF.

    Saturating in the setter means that the upcoming implementation for
    binary operators no longer have to worry about doing the right
    thing for -ffinite-math-only.  If the range goes outside the limits,
    it'll get chopped down.

Looking at the assembly code, test, testf and testl are all collapsing down to
calls to abort.
       .file   "k.c"
        .section P,"ax"
        .section        .text.unlikely,"ax",@progbits
        .global _test
        .type   _test, @function
_test:
        bsr     _abort
        .size   _test, .-_test
        .global _testf
        .type   _testf, @function
_testf:
        bsr     _abort
        .size   _testf, .-_testf
        .global _testl
        .type   _testl, @function
_testl:
        bsr     _abort
        .size   _testl, .-_testl
        .global _main
        .type   _main, @function
_main:
        bsr     _abort
        .size   _main, .-_main
        .ident  "GCC: (GNU) 13.0.0 20220923 (experimental)"

[Bug tree-optimization/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #1 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
The target seems to set flag_finite_math_only.  We are much more aggressive at
folding comparisons involving infinities with this flag.

config/rx/rx.cc:

      /* Alert the user if they are changing the optimization options
         to use IEEE compliant floating point arithmetic with RX FPU insns.  */
      if (TARGET_USE_FPU
          && !flag_finite_math_only)
        warning (0, "RX FPU instructions do not support NaNs and infinities");
    }

I think this test is invalid for this architecture.  XFAIL?

[Bug tree-optimization/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
             Target|                            |rx-elf

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Well, but the actual test compares Inf against 34 and even when Inf is not
supported the tests should succeed?

[Bug tree-optimization/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ah, probably the

void test(double f, double i)
{
...
  if (i != __builtin_inf())
    abort ();

int main()
{
  test (34.0, __builtin_inf());
...

test is "miscompiled", similar to what we see in povray.

We can probably gate parts of the testcase with

#if __FINITE_MATH_ONLY__ == 0

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> Ah, probably the
> 
> void test(double f, double i)
> {
> ...
>   if (i != __builtin_inf())
>     abort ();
> 
> int main()
> {
>   test (34.0, __builtin_inf());
> ...
> 
> test is "miscompiled", similar to what we see in povray.
> 
> We can probably gate parts of the testcase with
> 
> #if __FINITE_MATH_ONLY__ == 0

Yeah, I saw that.  Agreed that we should predicate parts of this testcase.

Also, it seems weird that the gimple representation is inconsistent between the
conditional and the call site.  Before einline we have:

void test (double f, double i)
{
  <bb 2> :
  if (i_1(D) <= 3.4028234663852885981170418348451692544e+38)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  abort ();

  <bb 4> :
  return;

}

int main ()
{
  <bb 2> :
  test (3.4e+1,  Inf);

}

Which the inliner through some unknown mechanism (to me), folds to a call to
abort in main.  This is before evrp or ethread.  I wonder how it folded that
with.  Anywho..that's neither here nor there.

What I'm concerned is that INF is being expanded differently between test and
main.  Shouldn't we at least be consistent?

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[jeffreyalaw at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #5 from Jeffrey A. Law <jeffreyalaw at gmail dot com> ---
First, let's not focus too much on that specific test.  There's others:

Tests that now fail, but worked before (20 tests):

rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -O1 
rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -O2 
rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -O2 -flto
-fno-use-linker-plugin -flto-partition=none 
rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects 
rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -O3 -g 
rx-sim: gcc.c-torture/execute/ieee/hugeval.c execution,  -Os 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -O1 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -O2 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -O2 -flto
-fno-use-linker-plugin -flto-partition=none 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -O3 -g 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -Og -g 
rx-sim: gcc.c-torture/execute/ieee/inf-2.c execution,  -Os 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -O1 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -O2 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -O2 -flto
-fno-use-linker-plugin -flto-partition=none 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -O3 -g 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -Og -g 
rx-sim: gcc.c-torture/execute/ieee/inf-3.c execution,  -Os 

Given the magic hackery going on inside the RX target files, I wonder if we
should skip the entire ieee suite for rx.

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jeffrey A. Law from comment #5)
> Given the magic hackery going on inside the RX target files, I wonder if we
> should skip the entire ieee suite for rx.

Doesn't add_options_for_ieee already handle RX though?
    if { [istarget rx-*-*] } {
       return "$flags -mnofpu"
    }
ieee.exp repeats some of the add_options_for_ieee stuff and more though, so
shouldn't we
--- gcc/testsuite/gcc.c-torture/execute/ieee/ieee.exp.jj        2022-01-11
23:11:22.969282018 +0100
+++ gcc/testsuite/gcc.c-torture/execute/ieee/ieee.exp   2022-12-02
12:15:22.307173074 +0100
@@ -52,6 +52,9 @@ if { [istarget "alpha*-*-*"]
      || [istarget "sh*-*-*"] } then {
   lappend additional_flags "-mieee"
 }
+if [istarget rx-*-*] then {
+  lappend additional_flags "-mnofpu"
+}

 if { ![check_effective_target_signal] } {
     lappend additional_flags "-DSIGNAL_SUPPRESS"
?

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org

--- Comment #7 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Yea, I think that's a better solution than simply disabling the whole ieee
directory on the rx.  It appears to DTRT for inf-2 which was the problematical
test.

Consider it pre-approved.

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:8872df353884fec6d7ff0e8bf8eee356439ec8d8

commit r13-4504-g8872df353884fec6d7ff0e8bf8eee356439ec8d8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Dec 6 11:39:51 2022 +0100

    testsuite: Use -mnofpu for rx-*-* in ieee testsuite [PR107046]

    add_options_for_ieee has:
        if { [istarget alpha*-*-*]
             || [istarget sh*-*-*] } {
           return "$flags -mieee"
        }
        if { [istarget rx-*-*] } {
           return "$flags -mnofpu"
        }
    but ieee.exp doesn't use add_options_for_ieee, instead it has:
    if { [istarget "alpha*-*-*"]
         || [istarget "sh*-*-*"] } then {
      lappend additional_flags "-mieee"
    }
    among other things (plus -ffloat-store on some arches etc.).

    The following patch adds the rx -mnofpu similarly in the hope
    of fixing ieee.exp FAILs on rx.

    Preapproved in the PR by Jeff, committed to trunk.

    2022-12-06  Jakub Jelinek  <jakub@redhat.com>

            PR testsuite/107046
            * gcc.c-torture/execute/ieee/ieee.exp: For rx-*-* append
            -mnofpu.

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed I assume.

[Bug testsuite/107046] [13 Regression] Recent FP range work causing inf-2 to be miscompiled on rx-elf

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107046

--- Comment #10 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Yes.  I should have changed the state on this BZ a few weeks back.