Bug 14547 - [3.4 Regression] Passing _Complex long double does not follow the ABI
Summary: [3.4 Regression] Passing _Complex long double does not follow the ABI
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 3.4.0
: P2 critical
Target Milestone: 3.4.0
Assignee: Richard Henderson
URL:
Keywords: ABI, patch, wrong-code
Depends on:
Blocks:
 
Reported: 2004-03-12 08:21 UTC by Richard Henderson
Modified: 2004-03-16 00:28 UTC (History)
2 users (show)

See Also:
Host:
Target: alpha*-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-03-12 15:29:56


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Henderson 2004-03-12 08:21:46 UTC
The SPLIT_COMPLEX_ARGS code added to 3.4 is not sufficiently general.  On Alpha,
_Complex float and _Complex double are supposed to be split, while _Complex long
double is passed by invisible reference, just like plain long double.

This ABI problem is noticed by gcc.dg/compat/scalar-by-value-3, since we do in
fact do the right thing in va_arg(), but can trivially be seen by examining the
code generated for 

   void foo(_Complex long double);
   void bar(void) { foo(0); }

Patch in progress...
Comment 1 GCC Commits 2004-03-12 10:03:39 UTC
Subject: Bug 14547

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rth@gcc.gnu.org	2004-03-12 10:03:32

Modified files:
	gcc/config/alpha: alpha.c alpha.h 
	gcc            : ChangeLog calls.c expr.h function.c system.h 
	                 target-def.h target.h 
	gcc/doc        : tm.texi 
	gcc/config/rs6000: rs6000.c rs6000.h 
	gcc/config/xtensa: xtensa.c xtensa.h 

Log message:
	PR target/14547
	* target.h (struct gcc_target): Move calls substructure before
	booleans.  Add split_complex_arg.
	* function.c (assign_parms, split_complex_args): Use it.
	* calls.c (expand_call): Likewise.
	(split_complex_values): Likewise.  Check for splittable types
	before allocating memory.
	(split_complex_types): Likewise.
	* system.h (SPLIT_COMPLEX_ARGS): Poison.
	* expr.h (SPLIT_COMPLEX_ARGS): Remove.
	* target-def.h (TARGET_SPLIT_COMPLEX_ARG): New.
	* config/alpha/alpha.c (alpha_split_complex_arg): New.
	(TARGET_SPLIT_COMPLEX_ARG): New.
	* config/alpha/alpha.h (SPLIT_COMPLEX_ARGS): Remove.
	* config/rs6000/rs6000.c (TARGET_SPLIT_COMPLEX_ARG): New.
	(rs6000_override_options): Zap it for non-AIX.
	(rs6000_function_value): Use targetm.calls.split_complex_arg.
	* config/rs6000/rs6000.h (SPLIT_COMPLEX_ARGS): Remove.
	* config/xtensa/xtensa.c (TARGET_SPLIT_COMPLEX_ARG): New.
	* config/xtensa/xtensa.h (SPLIT_COMPLEX_ARGS): Remove.
	* doc/tm.texi (TARGET_SPLIT_COMPLEX_ARG): Modify from old
	SPLIT_COMPLEX_ARGS entry.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/alpha/alpha.c.diff?cvsroot=gcc&r1=1.358&r2=1.359
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/alpha/alpha.h.diff?cvsroot=gcc&r1=1.218&r2=1.219
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3144&r2=2.3145
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/calls.c.diff?cvsroot=gcc&r1=1.324&r2=1.325
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.h.diff?cvsroot=gcc&r1=1.153&r2=1.154
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/function.c.diff?cvsroot=gcc&r1=1.503&r2=1.504
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/system.h.diff?cvsroot=gcc&r1=1.206&r2=1.207
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target-def.h.diff?cvsroot=gcc&r1=1.74&r2=1.75
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target.h.diff?cvsroot=gcc&r1=1.82&r2=1.83
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/doc/tm.texi.diff?cvsroot=gcc&r1=1.311&r2=1.312
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.c.diff?cvsroot=gcc&r1=1.605&r2=1.606
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.h.diff?cvsroot=gcc&r1=1.314&r2=1.315
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/xtensa/xtensa.c.diff?cvsroot=gcc&r1=1.53&r2=1.54
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/xtensa/xtensa.h.diff?cvsroot=gcc&r1=1.53&r2=1.54

Comment 2 Richard Henderson 2004-03-12 10:08:08 UTC
patch: http://gcc.gnu.org/ml/gcc-patches/2004-03/msg01016.html
Comment 3 Andrew Pinski 2004-03-12 15:29:55 UTC
Confirmed.
Comment 4 Mark Mitchell 2004-03-13 16:27:00 UTC
Subject: Re:  [3.4 Regression] Passing _Complex long double
 does not follow the ABI

rth at gcc dot gnu dot org wrote:

> ------- Additional Comments From rth at gcc dot gnu dot org  2004-03-12 10:08 -------
> patch: http://gcc.gnu.org/ml/gcc-patches/2004-03/msg01016.html
> 

The patch doesn't look quite right to me.

-  if (SPLIT_COMPLEX_ARGS)
+  /* If the target wants to split complex arguments into scalars, do 
so.  */
+  if (targetm.calls.split_complex_arg)
      fnargs = split_complex_args (fnargs);

Shouldn't that be "targetm.calls.split_complex_arg()"?

-  if (SPLIT_COMPLEX_ARGS && fnargs != orig_fnargs)
+  if (targetm.calls.split_complex_arg && fnargs != orig_fnargs)

Similarly?

Otherwise, if a backend defined this to be a function that always 
returned false, the compiler would treat that differently from the NULL 
pointer, which is (by the docs) supposed to be equivalent to the 
function that always returns false.

Comment 5 Richard Henderson 2004-03-13 19:18:36 UTC
Subject: Re:  [3.4 Regression] Passing _Complex long double does not follow the ABI

On Sat, Mar 13, 2004 at 08:26:47AM -0800, Mark Mitchell wrote:
> +  if (targetm.calls.split_complex_arg)
>      fnargs = split_complex_args (fnargs);
> 
> Shouldn't that be "targetm.calls.split_complex_arg()"?

No.  If the function pointer is null, it's defined to be false,
so we avoid calling it N times to find out that we don't need
to do anything.

> Otherwise, if a backend defined this to be a function that always 
> returned false, the compiler would treat that differently from the NULL 
> pointer, which is (by the docs) supposed to be equivalent to the 
> function that always returns false.

If the target does define the target hook non-null but always false,
then we'll go round the check-to-see-if-we-need-to-do-something loop
and discover the answer is no.  So while we would in fact do the right
thing under such a circumstance, we'd waste a bit of time doing so.



r~
Comment 6 Mark Mitchell 2004-03-15 16:14:07 UTC
Subject: Re:  [3.4 Regression] Passing _Complex long double
 does not follow the ABI

rth at redhat dot com wrote:

>------- Additional Comments From rth at redhat dot com  2004-03-13 19:18 -------
>Subject: Re:  [3.4 Regression] Passing _Complex long double does not follow the ABI
>
>On Sat, Mar 13, 2004 at 08:26:47AM -0800, Mark Mitchell wrote:
>  
>
>>+  if (targetm.calls.split_complex_arg)
>>     fnargs = split_complex_args (fnargs);
>>
>>Shouldn't that be "targetm.calls.split_complex_arg()"?
>>    
>>
>
>No.  If the function pointer is null, it's defined to be false,
>so we avoid calling it N times to find out that we don't need
>to do anything.
>
>  
>
Thanks for the explanation.  Patch OK for 3.4.0.

Comment 7 GCC Commits 2004-03-15 23:22:56 UTC
Subject: Bug 14547

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	rth@gcc.gnu.org	2004-03-15 23:22:51

Modified files:
	gcc            : ChangeLog calls.c expr.h function.c system.h 
	                 target-def.h target.h 
	gcc/config/alpha: alpha.c alpha.h 
	gcc/config/rs6000: rs6000.c rs6000.h 
	gcc/config/xtensa: xtensa.c xtensa.h 
	gcc/doc        : tm.texi 

Log message:
	PR target/14547
	* target.h (struct gcc_target): Move calls substructure before
	booleans.  Add split_complex_arg.
	* function.c (assign_parms, split_complex_args): Use it.
	* calls.c (expand_call): Likewise.
	(split_complex_values): Likewise.  Check for splittable types
	before allocating memory.
	(split_complex_types): Likewise.
	* system.h (SPLIT_COMPLEX_ARGS): Poison.
	* expr.h (SPLIT_COMPLEX_ARGS): Remove.
	* target-def.h (TARGET_SPLIT_COMPLEX_ARG): New.
	* config/alpha/alpha.c (alpha_split_complex_arg): New.
	(TARGET_SPLIT_COMPLEX_ARG): New.
	* config/alpha/alpha.h (SPLIT_COMPLEX_ARGS): Remove.
	* config/rs6000/rs6000.c (TARGET_SPLIT_COMPLEX_ARG): New.
	(rs6000_override_options): Zap it for non-AIX.
	(rs6000_function_value): Use targetm.calls.split_complex_arg.
	* config/rs6000/rs6000.h (SPLIT_COMPLEX_ARGS): Remove.
	* config/xtensa/xtensa.c (TARGET_SPLIT_COMPLEX_ARG): New.
	* config/xtensa/xtensa.h (SPLIT_COMPLEX_ARGS): Remove.
	* doc/tm.texi (TARGET_SPLIT_COMPLEX_ARG): Modify from old
	SPLIT_COMPLEX_ARGS entry.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.343&r2=2.2326.2.344
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/calls.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.315.2.4&r2=1.315.2.5
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.152.4.1&r2=1.152.4.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/function.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.483.4.9&r2=1.483.4.10
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/system.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.188.2.2&r2=1.188.2.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target-def.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.63.4.1&r2=1.63.4.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.74.2.2&r2=1.74.2.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/alpha/alpha.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.342.4.7&r2=1.342.4.8
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/alpha/alpha.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.213.2.5&r2=1.213.2.6
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.576.2.14&r2=1.576.2.15
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.306.4.2&r2=1.306.4.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/xtensa/xtensa.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.43.4.6&r2=1.43.4.7
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/xtensa/xtensa.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.43.4.5&r2=1.43.4.6
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/doc/tm.texi.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.281.2.12&r2=1.281.2.13

Comment 8 Richard Henderson 2004-03-16 00:24:24 UTC
Fixed.