[Bug middle-end/29253] New: expand_abs wrong default code for floating point

[Bug middle-end/29253] New: expand_abs wrong default code for floating point

[dje at gcc dot gnu dot org]

When expanding fabs inline, GCC attempts to use a sequence without jumps.  If
that sequence fails, GCC falls back to the equivalent of

if (x >= 0.0)
  return x;
return -x;

This sequence is incorrect for the IEEE floating point value -0.0.

This problem normally does not occur because most architectures have an
instruction to compute fabs or one can perform bit-twiddling, especially for
software floating point emulation.  However, for the cases such as software
floating point emulation of long double, the jump sequence is produced, leading
to wrong results for the corner case.


-- 
           Summary: expand_abs wrong default code for floating point
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dje at gcc dot gnu dot org
 GCC build triplet: *-*-*
  GCC host triplet: *-*-*
GCC target triplet: *-*-*


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2006-09-28 09:59 -------
Confirmed.  As expand_copysign, expand_fabs should defer to the library in this
case.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-09-28 09:59:18
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[steven at gcc dot gnu dot org]

------- Comment #2 from steven at gcc dot gnu dot org  2008-02-03 14:37 -------
Test case and HOWTO reproduce would be welcome.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[dje at gcc dot gnu dot org]

------- Comment #3 from dje at gcc dot gnu dot org  2008-02-04 14:27 -------
main ()
{
  if (fabs (-0.0) != 0.0)
    abort ();
}

compile with -msoft-float and optimization on powerpc-linux.


-- 

dje at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
   Last reconfirmed|2006-09-28 09:59:18         |2008-02-04 14:27:46
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2008-02-04 14:51 -------
Created an attachment (id=15093)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15093&action=view)
patch

This should fix it.  Can someone test it on ppc please?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2008-02-04 14:57 -------
Executable testcase:

extern double fabs(double);
extern void abort(void);
void __attribute__((noinline)) foo(double x)
{
  if (fabs (x) != 0.0)
    abort ();
}
int main ()
{
  foo(-0.0);
  return 0;
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2008-02-04 15:07 -------
Uh, and we fold ABS <x> != 0.0 to x != 0.0, which makes the testcase
uninteresting as well.  Also it happens that for double we succeed with
generating the nojump variant.  And I don't have a cross with long-double
enabled appearantly.  So, _possibly_ better testcase :)

extern long double fabsl(double);
extern void abort(void);
long double zero = 0.0L;
void __attribute__((noinline)) foo(long double x)
{
  if (fabsl (x) != zero)
    abort ();
}
int main ()
{
  foo(-0.0L);
  return 0;
}


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[bergner at gcc dot gnu dot org]

------- Comment #7 from bergner at gcc dot gnu dot org  2008-02-04 16:30 -------
I'm testing the patch from Comment #4 on powerpc64-linux.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[bergner at gcc dot gnu dot org]

------- Comment #8 from bergner at gcc dot gnu dot org  2008-02-04 17:04 -------
Ok, I bootstrapped with revision 132091.  The patched compiler ICE's on the
following test case:

typedef float TFtype __attribute__ ((mode (TF)));
TFtype
divtc3 (TFtype a, TFtype b)
{
  if (__builtin_fabsl (a) < __builtin_fabsl (b))
    {
      return a;
    }
  return b;
}

Here's a backtrace from the non debug build:

Program received signal SIGSEGV, Segmentation fault.
0x105475d4 in commutative_operand_precedence (op=0x0)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/rtlanal.c:2886
2886      enum rtx_code code = GET_CODE (op);
(gdb) bt
#0  0x105475d4 in commutative_operand_precedence (op=0x0)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/rtlanal.c:2886
#1  0x105478b8 in swap_commutative_operands_p (x=0x0, y=0x0)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/rtlanal.c:2951
#2  0x10212b1c in do_compare_rtx_and_jump (op0=0x0, op1=0x0, code=LT,
unsignedp=0, mode=TFmode, size=0x0, 
    if_false_label=0x0, if_true_label=0xf7f02b70)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/dojump.c:837
#3  0x10213edc in do_compare_and_jump (exp=0xf7f022a0, signed_code=LT,
unsigned_code=LTU, if_false_label=0x0, 
    if_true_label=0xf7f02b70) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/dojump.c:1014
#4  0x1020f4cc in do_jump (exp=0xf7f022a0, if_false_label=0x0,
if_true_label=0xf7f02b70)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/dojump.c:396
#5  0x1020c138 in jumpif (exp=0xf7f022a0, label=0xf7f02b70)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/dojump.c:109
#6  0x10b42a44 in expand_gimple_cond_expr (bb=0xf7f0f6c0, stmt=0xf7f02300)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/cfgexpand.c:1315
#7  0x10b43c44 in expand_gimple_basic_block (bb=0xf7f0f6c0)
    at /home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/cfgexpand.c:1578
#8  0x10b45dc0 in tree_expand_cfg () at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/cfgexpand.c:1921
[snip]


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2008-02-05 10:54 -------
Created an attachment (id=15098)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15098&action=view)
patch

Try this instead.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #15093|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[bergner at gcc dot gnu dot org]

------- Comment #10 from bergner at gcc dot gnu dot org  2008-02-05 16:38 -------
Bootstrap and regtesting is in progress on the new patch.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[bergner at gcc dot gnu dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3704 bytes --]



------- Comment #11 from bergner at gcc dot gnu dot org  2008-02-05 19:34 -------
Now we get a different ICE with the following test case:

bergner@etna:~/gcc/PR29253/bugs> cat _abs_r16.f95 
elemental function _gfortran_specific__abs_r16 (parm)
   real (kind=16), intent (in) :: parm
   real (kind=16) :: _gfortran_specific__abs_r16
   _gfortran_specific__abs_r16 = abs (parm)
end function
bergner@etna:~/gcc/PR29253/bugs>
/home/bergner/gcc/PR29253/build/gcc-mainline-patched-v2/./gcc/gfortran
-B/home/bergner/gcc/PR29253/build/gcc-mainline-patched-v2/./gcc/ 
-fallow-leading-underscore -O0 -msoft-float -c _abs_r16.f95
_abs_r16.f95: In function ‘_gfortran_specific__abs_r16’:
_abs_r16.f95:4: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


GDB backtrace:

Program received signal SIGSEGV, Segmentation fault.
build_fold_addr_expr (t=0x0) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/fold-const.c:7905
7905                                               build_pointer_type
(TREE_TYPE (t)),
(gdb) bt 5
#0  build_fold_addr_expr (t=0x0) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/fold-const.c:7905
#1  0x101d16bc in expand_expr_real_1 (exp=0xf7ed2920, target=0xf7ed2c60,
tmode=TFmode, modifier=EXPAND_NORMAL, 
    alt_rtl=<value optimized out>) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/expr.c:8757
#2  0x101d16bc in expand_expr_real_1 (exp=0xf7ed2920, target=0xf7ed2c60,
tmode=TFmode, modifier=EXPAND_NORMAL, 
    alt_rtl=<value optimized out>) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/expr.c:8757
#3  0x101d16bc in expand_expr_real_1 (exp=0xf7ed2920, target=0xf7ed2c60,
tmode=TFmode, modifier=EXPAND_NORMAL, 
    alt_rtl=<value optimized out>) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/expr.c:8757
#4  0x101d16bc in expand_expr_real_1 (exp=0xf7ed2920, target=0xf7ed2c60,
tmode=TFmode, modifier=EXPAND_NORMAL, 
    alt_rtl=<value optimized out>) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/expr.c:8757
(More stack frames follow...)
(gdb) frame 1
#1  0x101d16bc in expand_expr_real_1 (exp=0xf7ed2920, target=0xf7ed2c60,
tmode=TFmode, modifier=EXPAND_NORMAL, 
    alt_rtl=<value optimized out>) at
/home/bergner/gcc/PR29253/gcc-mainline-patched/gcc/expr.c:8757
8757              tree call = build_fold_addr_expr (mathfn_built_in (type,
BUILT_IN_FABS));
(gdb) ptree exp
 <abs_expr 0xf7ed2920
    type <real_type 0xf7e9ac40 real(kind=16) TF
        size <integer_cst 0xf7fd7870 constant invariant 128>
        unit size <integer_cst 0xf7fd78a0 constant invariant 16>
        align 128 symtab 0 alias set -1 canonical type 0xf7e9ac40 precision 128
        pointer_to_this <pointer_type 0xf7e9ad90> reference_to_this
<reference_type 0xf7ed19a0>>

    arg 0 <var_decl 0xf7ed1af0 D.602 type <real_type 0xf7e9ac40 real(kind=16)>
        used ignored TF file _abs_r16.f95 line 5 col 0 size <integer_cst
0xf7fd7870 128> unit size <integer_cst 0xf7fd78a0 16>
        align 128 context <function_decl 0xf7ed3600
_gfortran_specific__abs_r16>
        (reg:TF 121 [ D.602 ])
        chain <var_decl 0xf7ed1b60 __result__gfortran_specific__.0 type
<real_type 0xf7e9ac40 real(kind=16)>
            used ignored TF file _abs_r16.f95 line 5 col 0 size <integer_cst
0xf7fd7870 128> unit size <integer_cst 0xf7fd78a0 16>
            align 128 context <function_decl 0xf7ed3600
_gfortran_specific__abs_r16>
            (reg:TF 120 [ __result__gfortran_specific__.0 ]) chain <var_decl
0xf7ed1bd0 D.604>>>>     
(gdb) pr target
(reg:TF 120 [ __result__gfortran_specific__.0 ])


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29253

[Bug middle-end/29253] expand_abs wrong default code for floating point

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2008-02-04 14:27:46         |2021-8-8

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is still true as far as I can tell by reading the code.
The case where it will happen is REAL_MODE_FORMAT does not have a simple sign
bit or 

GET_MODE_SIZE (mode) <= UNITS_PER_WORD && the mode does not exist

Which I think the first is true for IBM 128bit long doubles.

Note I don't know if IBM 128bit long double is an important use case.

[Bug middle-end/29253] expand_abs wrong default code for floating point

[pinskia at gcc dot gnu.org]

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

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55235
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55235&action=edit
Slightly different patch

This is a slightly different patch but it basically does the same except it
uses emit_library_call_value instead of creating trees to the expanding.

[Bug middle-end/29253] expand_abs wrong default code for floating point

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55235|0                           |1
        is obsolete|                            |

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55236
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55236&action=edit
Updated patch which I did some light testing on powerpc64-linux-gnu (with
-msoft-float)

I updated the patch because I noticed that we need to move the result of the
libcall to a temp register otherwise it might be removed.

I tested all of the testcases in this bug report with  -mlong-double-128
-msoft-float and we get the call to absl in the assembly output and they all
look correct.