Bug 29253 - expand_abs wrong default code for floating point
Summary: expand_abs wrong default code for floating point
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2006-09-27 16:37 UTC by David Edelsohn
Modified: 2008-02-05 19:34 UTC (History)
4 users (show)

See Also:
Host: *-*-*
Target: *-*-*
Build: *-*-*
Known to work:
Known to fail:
Last reconfirmed: 2008-02-04 14:27:46


Attachments
patch (752 bytes, patch)
2008-02-04 14:51 UTC, Richard Biener
Details | Diff
patch (1.17 KB, patch)
2008-02-05 10:54 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Edelsohn 2006-09-27 16:37:07 UTC
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.
Comment 1 Richard Biener 2006-09-28 09:59:18 UTC
Confirmed.  As expand_copysign, expand_fabs should defer to the library in this case.
Comment 2 Steven Bosscher 2008-02-03 14:37:37 UTC
Test case and HOWTO reproduce would be welcome.
Comment 3 David Edelsohn 2008-02-04 14:27:46 UTC
main ()
{
  if (fabs (-0.0) != 0.0)
    abort ();
}

compile with -msoft-float and optimization on powerpc-linux.
Comment 4 Richard Biener 2008-02-04 14:51:19 UTC
Created attachment 15093 [details]
patch

This should fix it.  Can someone test it on ppc please?
Comment 5 Richard Biener 2008-02-04 14:57:11 UTC
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;
}
Comment 6 Richard Biener 2008-02-04 15:07:39 UTC
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;
}
Comment 7 Peter Bergner 2008-02-04 16:30:11 UTC
I'm testing the patch from Comment #4 on powerpc64-linux.
Comment 8 Peter Bergner 2008-02-04 17:04:43 UTC
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]
Comment 9 Richard Biener 2008-02-05 10:54:34 UTC
Created attachment 15098 [details]
patch

Try this instead.
Comment 10 Peter Bergner 2008-02-05 16:38:52 UTC
Bootstrap and regtesting is in progress on the new patch.
Comment 11 Peter Bergner 2008-02-05 19:34:43 UTC
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 ])