Bug 44632 - [4.4/4.5 regression] wrong code for complex division
Summary: [4.4/4.5 regression] wrong code for complex division
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.4
: P4 normal
Target Milestone: 4.4.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2010-06-22 13:58 UTC by Matthias Klose
Modified: 2010-08-16 20:27 UTC (History)
3 users (show)

See Also:
Host:
Target: hppa-linux-gnu
Build:
Known to work: 4.3.5 4.6.0
Known to fail: 4.4.4 4.5.0
Last reconfirmed: 2010-08-07 19:13:51


Attachments
test.ii.gz (69.83 KB, application/x-gunzip)
2010-08-07 19:59 UTC, dave
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2010-06-22 13:58:07 UTC
[forwarded from http://bugs.debian.org/585925]

seen with current 4.5 and 4.5 branches, and trunk

g++-4.4 miscompiles complex division:

(sid)jwilk@paer:~$ cat test.cxx 
#include <complex>
#include <iostream>

void f(std::complex<double> x)
{
     std::cout << x << std::endl;
     x = 1.0 / x;
     std::cout << x << std::endl;
}

int main()
{
     f(2.0);
}

(sid)jwilk@paer:~$ g++-4.3 -Wall test.cxx && ./a.out
(2,0)
(0.5,0)

(sid)jwilk@paer:~$ g++-4.4 -Wall test.cxx && ./a.out
(2,0)
(1,0)
Comment 1 John David Anglin 2010-08-07 19:32:43 UTC
Starting program: /home/dave/gnu/gcc-4.6/objdir/gcc/testsuite/g++/test 
(2,0)

Breakpoint 2, 0x000109f8 in f (x=...) at test.cxx:12
12	     x = 1.0 / x;
(gdb) step
std::operator/<double> (__x=@0xc0246388, __y=...)
    at /home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/complex:429
429	      complex<_Tp> __r = __x;
(gdb) p &__y
$15 = (const std::complex<double> *) 0xc0246390
(gdb) p &__r
$16 = (std::complex<double> *) 0xc0246390
(gdb) p __y
$17 = (const std::complex<double> &) @0xc0246390: {_M_value = 2 + 0 * I}
(gdb) p __r
$18 = {_M_value = 2 + 0 * I}
(gdb) step
std::complex<double>::complex (this=0xc0246390, __r=1, __i=0)
    at /home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/complex:1172
1172		__real__ _M_value = __r;
(gdb) 
1173		__imag__ _M_value = __i;
(gdb) 
1174	      }
(gdb) 
std::operator/<double> (__x=@0xc0246388, __y=...)
    at /home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/complex:430
430	      __r /= __y;
(gdb) p __y
$19 = (const std::complex<double> &) @0xc0246390: {_M_value = 1 + 0 * I}
(gdb) p __r
$20 = {_M_value = 1 + 0 * I}

The problem is __r and __y share the same location.
Comment 2 John David Anglin 2010-08-07 19:52:10 UTC
With slightly modified test,

#include <complex>
#include <iostream>

void g(std::complex<double> x)
{
     std::cout << x << std::endl;
}

void f(std::complex<double> x)
{
     g (x);
     x = 1.0 / x;
     g (x);
}

int main()
{
     f(2.0);
}

it appears RTL is wrong at expansion:

;; x.1 = std::operator/<double> (&D.24646, &x.1); [return slot optimization]

(insn 18 17 19 (set (reg:SI 102)
        (plus:SI (reg/f:SI 90 virtual-stack-vars)
            (const_int 8 [0x8]))) test.cxx:12 -1
     (nil))
(insn 19 18 20 (set (reg:SI 103)
        (plus:SI (reg/f:SI 90 virtual-stack-vars)
            (const_int 8 [0x8]))) test.cxx:12 -1
     (nil))

(insn 20 19 21 (set (reg:SI 28 %r28)
        (reg:SI 103)) test.cxx:12 -1
     (nil))

(insn 21 20 22 (set (reg:SI 26 %r26)
        (reg/f:SI 90 virtual-stack-vars)) test.cxx:12 -1
     (nil))

(insn 22 21 23 (set (reg:SI 25 %r25)
        (reg:SI 102)) test.cxx:12 -1
     (nil))

(call_insn 23 22 0 (parallel [
            (call (mem:SI (symbol_ref/v/i:SI ("@_ZStdvIdESt7complexIT_ERKS1_RKS2_") [flags 0x1]  <function_decl 0x40f5d600 operator/>) [0 S4 A32])
                (const_int 16 [0x10]))
            (clobber (reg:SI 1 %r1))
            (clobber (reg:SI 2 %r2))
            (use (const_int 0 [0]))
        ]) test.cxx:12 -1
     (nil)
    (expr_list:REG_DEP_TRUE (use (reg:SI 25 %r25))
        (expr_list:REG_DEP_TRUE (use (reg:SI 26 %r26))
            (expr_list:REG_DEP_TRUE (use (reg:SI 28 %r28))
                (nil)))))

Comment 3 John David Anglin 2010-08-07 19:58:20 UTC
Richard do you know what's wrong?  I think the issue is the return slot
optimization.
Comment 4 dave 2010-08-07 19:59:40 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong
	code for complex division

Attached .ii.

Comment 5 dave 2010-08-07 19:59:40 UTC
Created attachment 21432 [details]
test.ii.gz
Comment 6 Richard Biener 2010-08-07 20:36:47 UTC
The argument should have prevented return slot optimization here.

;; x.1 = std::operator/<double> (&D.24646, &x.1); [return slot optimization]

Isn't this fixed on trunk since

2010-07-26  Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/43784
        * tree-nrv.c (dest_safe_for_nrv_p): It's not safe to NRV
        if the destination is used by the call.

?  Well, I'll have a look.
Comment 7 Richard Biener 2010-08-07 20:39:01 UTC
Btw, does this only happen at -O0?  If you adjust the testcase like

#include <complex>
#include <iostream>

void __attribute__((noinline))
g(std::complex<double> x)
{
  std::cout << x << std::endl;
}

void __attribute__((noinline))
f(std::complex<double> x)
{
  g (x);
  x = 1.0 / x;
  g (x);
}

int main()
{
  f(2.0);
}
Comment 8 Richard Biener 2010-08-07 20:40:27 UTC
On i?86-linux I see

<bb 2>:
  g (x);
  D.24518 = 1.0e+0;
  x = std::operator/<double> (&D.24518, &x);
  g (x);

so no return-slot optimization.  So I guess it has something to do with
the callee-copy thing of the x argument to this function.  Building a cross ...
Comment 9 dave 2010-08-07 20:57:41 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong code for complex division

> ;; x.1 = std::operator/<double> (&D.24646, &x.1); [return slot optimization]
> 
> Isn't this fixed on trunk since
> 
> 2010-07-26  Richard Guenther  <rguenther@suse.de>
> 
>         PR tree-optimization/43784
>         * tree-nrv.c (dest_safe_for_nrv_p): It's not safe to NRV
>         if the destination is used by the call.

Still present on trunk today.

Comment 10 dave 2010-08-07 21:00:40 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong code for complex division

> Btw, does this only happen at -O0?  If you adjust the testcase like

No, it also fails at -O2 where the entire computation is inline.

Dave
Comment 11 dave 2010-08-07 21:04:06 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong code for complex division

> On i?86-linux I see
> 
> <bb 2>:
>   g (x);
>   D.24518 = 1.0e+0;
>   x = std::operator/<double> (&D.24518, &x);
>   g (x);
> 
> so no return-slot optimization.  So I guess it has something to do with
> the callee-copy thing of the x argument to this function.  Building a cross ...

On hppa-linux, I see at -O0

  x.1 = x;
  g (x.1);
  D.24646 = 1.0e+0;
  x.1 = std::operator/<double> (&D.24646, &x.1); [return slot optimization]
  g (x.1);

Comment 12 Richard Biener 2010-08-07 21:16:09 UTC
Ok, I see when gimplifying the call that we mark it for NRV because while
x doesn't have it's address taken it's value-expr has and we didn't replace
it at that point but we check

4262		      else if (!is_gimple_non_addressable (*to_p))
4271			use_target = true;

so I think this is a bug that it doesn't mark the original parameter
decl as address-taken which happens here (function.c:gimplify_parameters):

                  /* If PARM was addressable, move that flag over
                     to the local copy, as its address will be taken,
                     not the PARMs.  */
                  if (TREE_ADDRESSABLE (parm))
                    {
                      TREE_ADDRESSABLE (parm) = 0;
                      TREE_ADDRESSABLE (local) = 1;
                    }

We should defer clearing the param addressable flag to update_address_taken.

So the following should fix this.  Can you bootstrap/test this?

Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 162781)
+++ gcc/function.c	(working copy)
@@ -3423,12 +3423,10 @@ gimplify_parameters (void)
 		  DECL_IGNORED_P (local) = 0;
 		  /* If PARM was addressable, move that flag over
 		     to the local copy, as its address will be taken,
-		     not the PARMs.  */
+		     not the PARMs.  Keep the parms address taken
+		     as we'll query that flag during gimplification.  */
 		  if (TREE_ADDRESSABLE (parm))
-		    {
-		      TREE_ADDRESSABLE (parm) = 0;
-		      TREE_ADDRESSABLE (local) = 1;
-		    }
+		    TREE_ADDRESSABLE (local) = 1;
 		}
 	      else
 		{

(patch to the 4.5 branch, but should apply to the trunk as well).
Comment 13 dave 2010-08-07 21:36:11 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong code for complex division

> So the following should fix this.  Can you bootstrap/test this?

Testing.
Comment 14 dave 2010-08-09 11:35:33 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong
	code for complex division

On Sat, 07 Aug 2010, rguenth at gcc dot gnu dot org wrote:

> So the following should fix this.  Can you bootstrap/test this?

Test results are here:
http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg00900.html

The following might be a regression:
Executing on host: /home/dave/gnu/gcc/objdir/./gcc/g++ -shared-libgcc -B/home/dave/gnu/gcc/objdir/./gcc -nostdinc++ -L/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/src -L/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/src/.libs -B/home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linux/bin/ -B/home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linux/lib/ -isystem /home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linux/include -isystem /home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linux/sys-include -B/home/dave/gnu/gcc/objdir/hppa-linux/./libstdc++-v3/src/.libs -g -O2 -D_GLIBCXX_ASSERT -fmessage-length=0 -ffunction-sections -fdata-sections -g -O2 -D_GNU_SOURCE -g -O2 -D_GNU_SOURCE -DLOCALEDIR="." -nostdinc++ -I/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/hppa-linux -I/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include -I/home/dave/gnu/gcc/gcc/libstdc++-v3/libsupc++ -I/home/dave/gnu/gcc/gcc/libstdc++-v3/include/backward -I/home/dave/gnu/gcc/gcc/libstdc++-v3/testsuite/util /home/dave/gnu/gcc/gcc/libstdc++-v3/testsuite/tr1/5_numerical_facilities/special_functions/01_assoc_laguerre/check_nan.cc    -include bits/stdc++.h ./libtestc++.a -Wl,--gc-sections  -lm   -o ./check_nan.exe    (timeout = 600)
In file included from /home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/tr1/cmath:95:0,                 from /home/dave/gnu/gcc/gcc/libstdc++-v3/testsuite/tr1/5_numerical_facilities/special_functions/01_assoc_laguerre/check_nan.cc:25:/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/tr1/poly_laguerre.tcc: In function '_Tp std::tr1::__detail::__poly_laguerre_large_n(unsigned int, _Tpa, _Tp) [with _Tpa = unsigned int, _Tp = float]':/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/tr1/poly_laguerre.tcc:106:5: internal compiler error: in simplify_subreg, at simplify-rtx.c:5129

Comment 15 dave 2010-08-09 11:37:00 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong

> 
> On Sat, 07 Aug 2010, rguenth at gcc dot gnu dot org wrote:
> 
> > So the following should fix this.  Can you bootstrap/test this?

Oh, I forgot to say test.cxx testcase is fixed.

Comment 16 dave 2010-08-09 11:44:02 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong

> The following might be a regression:
> Executing on host: /home/dave/gnu/gcc/objdir/./gcc/g++ -shared-libgcc -B/ho=
> me/dave/gnu/gcc/objdir/./gcc -nostdinc++ -L/home/dave/gnu/gcc/objdir/hppa-l=
> inux/libstdc++-v3/src -L/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/s=
> rc/.libs -B/home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linux/bin/ -B/home/dave/op=
> t/gnu/gcc/gcc-4.6.0/hppa-linux/lib/ -isystem /home/dave/opt/gnu/gcc/gcc-4.6=
> =2E0/hppa-linux/include -isystem /home/dave/opt/gnu/gcc/gcc-4.6.0/hppa-linu=
> x/sys-include -B/home/dave/gnu/gcc/objdir/hppa-linux/./libstdc++-v3/src/.li=
> bs -g -O2 -D_GLIBCXX_ASSERT -fmessage-length=3D0 -ffunction-sections -fdata=
> -sections -g -O2 -D_GNU_SOURCE -g -O2 -D_GNU_SOURCE -DLOCALEDIR=3D"." -nost=
> dinc++ -I/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/hppa-lin=
> ux -I/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include -I/home/dave=
> /gnu/gcc/gcc/libstdc++-v3/libsupc++ -I/home/dave/gnu/gcc/gcc/libstdc++-v3/i=
> nclude/backward -I/home/dave/gnu/gcc/gcc/libstdc++-v3/testsuite/util /home/=
> dave/gnu/gcc/gcc/libstdc++-v3/testsuite/tr1/5_numerical_facilities/special_=
> functions/01_assoc_laguerre/check_nan.cc    -include bits/stdc++.h ./libtes=
> tc++.a -Wl,--gc-sections  -lm   -o ./check_nan.exe    (timeout =3D 600)
> In file included from /home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/inc=
> lude/tr1/cmath:95:0,                 from /home/dave/gnu/gcc/gcc/libstdc++-=
> v3/testsuite/tr1/5_numerical_facilities/special_functions/01_assoc_laguerre=
> /check_nan.cc:25:/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/=
> tr1/poly_laguerre.tcc: In function '_Tp std::tr1::__detail::__poly_laguerre=
> _large_n(unsigned int, _Tpa, _Tp) [with _Tpa =3D unsigned int, _Tp =3D floa=
> t]':/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/tr1/poly_lagu=
> erre.tcc:106:5: internal compiler error: in simplify_subreg, at simplify-rt=
> x.c:5129

No, it is present without change.

Should the fix be backported?

Comment 17 rguenther@suse.de 2010-08-09 11:51:11 UTC
Subject: Re:  [4.4/4.5/4.6 regression] wrong
 code for complex division

On Mon, 9 Aug 2010, dave at hiauly1 dot hia dot nrc dot ca wrote:

> ------- Comment #16 from dave at hiauly1 dot hia dot nrc dot ca  2010-08-09 11:44 -------
> Subject: Re:  [4.4/4.5/4.6 regression] wrong
> 
> > t]':/home/dave/gnu/gcc/objdir/hppa-linux/libstdc++-v3/include/tr1/poly_lagu=
> > erre.tcc:106:5: internal compiler error: in simplify_subreg, at simplify-rt=
> > x.c:5129
> 
> No, it is present without change.
> 
> Should the fix be backported?

Probably yes.  I'm testing the fix on x86_64-linux now together with
a testcase and will apply it to trunk.  If you can do a bootstrap
and regtest on the 4.4 branch as well that would be nice.

Thx.
Comment 18 Richard Biener 2010-08-09 13:18:24 UTC
Subject: Bug 44632

Author: rguenth
Date: Mon Aug  9 13:18:08 2010
New Revision: 163031

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163031
Log:
2010-08-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/44632
	* function.c (gimplify_parameters): Do not clear addressable
	bit of the original parameter.

	* g++.dg/opt/nrv17.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/opt/nrv17.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/function.c
    trunk/gcc/testsuite/ChangeLog

Comment 19 Richard Biener 2010-08-09 13:31:19 UTC
Backports are pre-approved if they pass bootstrap & testing.
Comment 20 John David Anglin 2010-08-16 20:18:23 UTC
Subject: Bug 44632

Author: danglin
Date: Mon Aug 16 20:18:08 2010
New Revision: 163284

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163284
Log:
	Backport from mainline:
	2010-08-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/44632
	* function.c (gimplify_parameters): Do not clear addressable
	bit of the original parameter.


Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/function.c

Comment 21 John David Anglin 2010-08-16 20:25:09 UTC
Subject: Bug 44632

Author: danglin
Date: Mon Aug 16 20:24:54 2010
New Revision: 163285

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163285
Log:
	Backport from mainline:
	2010-08-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/44632
	* function.c (gimplify_parameters): Do not clear addressable
	bit of the original parameter.


Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/function.c

Comment 22 John David Anglin 2010-08-16 20:27:34 UTC
Fixed.