[RFC] math/testtgmath2: Fix breakage on fabs Vldouble1 due to optimization

  I have been testing with GCC trunk and GLIBC master while working on
OpenRISC ports.  This test has been failing which fabs not being called,
I am guessing this is due to new optimizations in GCC that are causing
the Vldouble1 call to also be optimized out now, but I may be wrong.

No other tests in math/* are failing for me exept for this one now.

  FAIL: math/test-tgmath2
  original exit status 1
  wrong function called, fabs (ldouble) failure on line 174

Note: I also added some details to the FAIL line to help track what issue caused
the failure.

---

Has anyone else seen this?

 math/test-tgmath2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

On Fri, 8 Jan 2021, Stafford Horne via Libc-alpha wrote:

> I have been testing with GCC trunk and GLIBC master while working on
> OpenRISC ports.  This test has been failing which fabs not being called,
> I am guessing this is due to new optimizations in GCC that are causing
> the Vldouble1 call to also be optimized out now, but I may be wrong.

This test is built with -fno-builtin.  If a function call is optimized 
out, that sounds like a GCC bug which should be fixed there.
  

On Fri, Jan 8, 2021, 7:53 AM Joseph Myers <joseph@codesourcery.com> wrote:

> On Fri, 8 Jan 2021, Stafford Horne via Libc-alpha wrote:
>
> > I have been testing with GCC trunk and GLIBC master while working on
> > OpenRISC ports.  This test has been failing which fabs not being called,
> > I am guessing this is due to new optimizations in GCC that are causing
> > the Vldouble1 call to also be optimized out now, but I may be wrong.
>
> This test is built with -fno-builtin.  If a function call is optimized
> out, that sounds like a GCC bug which should be fixed there.
>

(Replying on phone)

Thanks, that gives me a clue.

-stafford

>
  

On Thu, Jan 07, 2021 at 10:53:04PM +0000, Joseph Myers wrote:
> On Fri, 8 Jan 2021, Stafford Horne via Libc-alpha wrote:
> 
> > I have been testing with GCC trunk and GLIBC master while working on
> > OpenRISC ports.  This test has been failing which fabs not being called,
> > I am guessing this is due to new optimizations in GCC that are causing
> > the Vldouble1 call to also be optimized out now, but I may be wrong.
> 
> This test is built with -fno-builtin.  If a function call is optimized 
> out, that sounds like a GCC bug which should be fixed there.
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

Hello,

I did some more investigation but I can not quite understand what is going wrong
here.

When I compile with or1k-smh-linux-gnu-gcc -fdump-tree-all-all, I can see that
the call to fabs is being removed during the FRE (Full redundancy elimination)
pass [note 3].  This looks to be because ldouble and double are the same on my
architecture, this means the call to "fabs (Vdouble1)" and "fabs (Vldouble1)"
become the same thing and the compiler removes the second redundant call (with
-Og).

You mention -fno-builtin, but I still see __builtin_tgmath being used which is
what is used in tgmath.h [note 4].  Perhaps I am misunderstanding but
-fno-builtin does not elliminate the usage of __builtin_tgmath.


That said, I think the compiler is correct but my patch is wrong. I will try to
think of something else.


Notes:

1. Compile command line:
 or1k-smh-linux-gnu-gcc -mcmov -mror -mrori -msfimm -mshftimm -msext \
   -B/home/shorne/work/gnu-toolchain/local/bin/ test-tgmath2.c -c \
   -std=gnu11 -fgnu89-inline  \
   -g -Og -Wall -Wwrite-strings -Wundef -fmerge-all-constants \
   -frounding-math -fno-stack-protector -Wstrict-prototypes \
   -Wold-style-definition -fmath-errno \
   -fno-builtin \
   -DNO_LONG_DOUBLE \
   -I../include  ...
   -DMODULE_NAME=testsuite -include ../include/libc-symbols.h \
   -DTOP_NAMESPACE=glibc -I../soft-fp \
   -o /home/shorne/work/gnu-toolchain/build-glibc/math/test-tgmath2.o \
   -fdump-tree-all-all

2. I changed the test_fabs function to just call:

  TEST (fabs (vcldouble1), ldouble, cabs);
  TEST (fabs (Vfloat1), float, fabs);
  TEST (fabs (Vdouble1), double, fabs);
  TEST (fabs (Vldouble1), ldouble, fabs);

3. In the tree dump I see:

grep 'fabs' ../../build-glibc/math/test-tgmath2.c.03*t.*

../../build-glibc/math/test-tgmath2.c.034t.ethread:

    ;; Function fabs (fabs, funcdef_no=28, decl_uid=1116, cgraph_uid=29, symbol_order=84)
    doubleD.25 fabs (doubleD.25 xD.25027)
    ;; Function fabsf (fabsf, funcdef_no=41, decl_uid=1550, cgraph_uid=42, symbol_order=97)
    floatD.24 fabsf (floatD.24 xD.25074)
    ;; Function test_fabs (test_fabs, funcdef_no=13, decl_uid=5158, cgraph_uid=14, symbol_order=69)
    intD.1 test_fabs (const intD.1 Vint4D.5156, const long long intD.10 Vllong4D.5157)
      _9 = fabsfD.1550 (1.0e+0);
      _14 = fabsD.1116 (1.0e+0);
      _19 = fabsD.1116 (1.0e+0);
      _31 = test_fabsD.5158 (vint1.3287_3, vllong1.3288_4);

../../build-glibc/math/test-tgmath2.c.037t.fre1:
    ;; Function fabs (fabs, funcdef_no=28, decl_uid=1116, cgraph_uid=29, symbol_order=84)
    doubleD.25 fabs (doubleD.25 xD.25027)
    ;; Function fabsf (fabsf, funcdef_no=41, decl_uid=1550, cgraph_uid=42, symbol_order=97)
    floatD.24 fabsf (floatD.24 xD.25074)
    ;; Function test_fabs (test_fabs, funcdef_no=13, decl_uid=5158, cgraph_uid=14, symbol_order=69)
    Value numbering stmt = _9 = fabsf (1.0e+0);
    Value numbering stmt = _14 = fabs (1.0e+0);
    Value numbering stmt = _19 = fabs (1.0e+0);
    Replaced fabs (1.0e+0) with _14 in all uses of _19 = fabs (1.0e+0);
    Removing dead stmt _19 = fabs (1.0e+0);
    intD.1 test_fabs (const intD.1 Vint4D.5156, const long long intD.10 Vllong4D.5157)
      _9 = fabsfD.1550 (1.0e+0);
      _14 = fabsD.1116 (1.0e+0);
    Value numbering stmt = _31 = test_fabs (vint1.3287_3, vllong1.3288_4);
      _31 = test_fabsD.5158 (vint1.3287_3, vllong1.3288_4);

4. If I look at the expressions for the above:

  printfD.4229 ("%s failure on line %d\n", "wrong function called, cabs (ldouble) - __builtin_tgmath (fabsf, fabs, fabsl, fabsf32, fabsf64, fabsf32x, cabsf, cabs, cabsl, cabsf32, cabsf64, cabsf32x, ((vcldouble1)))", 173);
  printfD.4229 ("%s failure on line %d\n", "wrong function called, fabs (float) - __builtin_tgmath (fabsf, fabs, fabsl, fabsf32, fabsf64, fabsf32x, cabsf, cabs, cabsl, cabsf32, cabsf64, cabsf32x, ((Vfloat1)))", 174);
  printfD.4229 ("%s failure on line %d\n", "wrong function called, fabs (double) - __builtin_tgmath (fabsf, fabs, fabsl, fabsf32, fabsf64, fabsf32x, cabsf, cabs, cabsl, cabsf32, cabsf64, cabsf32x, ((Vdouble1)))", 175);
  printfD.4229 ("%s failure on line %d\n", "wrong function called, fabs (ldouble) - __builtin_tgmath (fabsf, fabs, fabsl, fabsf32, fabsf64, fabsf32x, cabsf, cabs, cabsl, cabsf32, cabsf64, cabsf32x, ((Vldouble1)))", 176);

-Stafford
  

On Sat, 9 Jan 2021, Stafford Horne via Libc-alpha wrote:

> When I compile with or1k-smh-linux-gnu-gcc -fdump-tree-all-all, I can see that
> the call to fabs is being removed during the FRE (Full redundancy elimination)
> pass [note 3].  This looks to be because ldouble and double are the same on my
> architecture, this means the call to "fabs (Vdouble1)" and "fabs (Vldouble1)"
> become the same thing and the compiler removes the second redundant call (with
> -Og).

I'd expect calls to fabs and fabsl, not two calls to fabs, once the front 
end has done the tgmath.h processing (given a GCC 8 or later compiler so 
it's not based on sizeof, anyway) and before any GIMPLE optimizations have 
run.

> You mention -fno-builtin, but I still see __builtin_tgmath being used which is
> what is used in tgmath.h [note 4].  Perhaps I am misunderstanding but
> -fno-builtin does not elliminate the usage of __builtin_tgmath.

__builtin_tgmath is a syntax construct (not actually a built-in function) 
that only exists at front-end level.  -fno-builtin should mean that the 
compiler doesn't know that fabs and fabsl do the same thing.  So the first 
question is what the calls look like before any GIMPLE optimizations.

Is the issue that the asm redirections in the headers mean that the call 
to fabsl is set up to call fabs at the assembler level and the compiler is 
thus noticing they are the same function, based on the asm redirection 
rather than on being built-in?  If so, maybe the test should be set up so 
that double and long double calls always use different function arguments 
to avoid such an optimization.