public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
@ 2016-09-28  1:20 Alan Modra
  2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Alan Modra @ 2016-09-28  1:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: Segher Boessenkool

Extend this attribute to cover long double ABIs, for 64-bit too.  The
idea is that the linker should warn if you are linking object files
with incompatible ABIs, for example IEEE128 long double with IBM long
double.  It's only a warning, and doesn't catch everything.  In
particular, global data mismatches.  ie. initialised global variables
in one format can be used by code that expects a different format
without warning.  Also, a function returning "sizeof (long double)"
or similar won't cause an object file to be tagged.  Oh, and you need
a new linker to see long double warnings.

The patch also corrects an error that crept in to code setting
rs6000_passes_float.  See the added comment.  Passing IEEE128 values
in vsx regs ought to set both Tag_GNU_Power_ABI_FP and
Tag_GNU_Power_ABI_Vector.

I've also added a new option, default on, that disables output of
.gnu_attribute tags.  That's needed because this patch alone
introduces libstdc++ testsuite regressions, fixed by the next patch.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux biarch.  OK to apply?

	* config/rs6000/sysv4.opt (mgnu-attr): New option.
	* config/rs6000/rs6000.c (HAVE_LD_PPC_GNU_ATTR): Define.
	(rs6000_passes_float): Comment.
	(rs6000_passes_long_double): New static var.
	(call_ABI_of_interest): Return false unless rs6000_gnu_attr is set.
	(init_cumulative_args): Set up to emit fp .gnu.attribute for
	ELF 64-bit ABIs as well as 32-bit ELF.  Correct rs6000_passes_float
	to include fp values returned in vectors.
	Set rs6000_passes_long_double.
	(rs6000_function_arg_advance_1): Likewise for function args.
	(rs6000_elf_file_end): Emit fp .gnu.attribute for ELF 64-bit ABIs,
	and SPE.  Emit long double tag value too.
	(rs6000_opt_vars): Add gnu-attr.
	* configure.ac (HAVE_LD_PPC_GNU_ATTR): New ppc32 test.
	* configure: Regenerate.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 4d3706c..49f662a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -183,8 +183,16 @@ unsigned rs6000_pmode;
 unsigned rs6000_pointer_size;
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-/* Flag whether floating point values have been passed/returned.  */
+# ifndef HAVE_LD_PPC_GNU_ATTR
+# define HAVE_LD_PPC_GNU_ATTR 0
+# endif
+/* Flag whether floating point values have been passed/returned.
+   Note that this doesn't say whether fprs are used, since the
+   Tag_GNU_Power_ABI_FP .gnu.attributes value this flag controls
+   should be set for soft-float values passed in gprs and ieee128
+   values passed in vsx registers.  */
 static bool rs6000_passes_float;
+static bool rs6000_passes_long_double;
 /* Flag whether vector values have been passed/returned.  */
 static bool rs6000_passes_vector;
 /* Flag whether small (<= 8 byte) structures have been returned.  */
@@ -10920,7 +10928,7 @@ rs6000_return_in_msb (const_tree valtype)
 static bool
 call_ABI_of_interest (tree fndecl)
 {
-  if (symtab->state == EXPANSION)
+  if (rs6000_gnu_attr && symtab->state == EXPANSION)
     {
       struct cgraph_node *c_node;
 
@@ -10997,7 +11005,7 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree fntype,
     }
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-  if (DEFAULT_ABI == ABI_V4)
+  if (TARGET_ELF && (TARGET_64BIT || DEFAULT_ABI == ABI_V4))
     {
       cum->escapes = call_ABI_of_interest (fndecl);
       if (cum->escapes)
@@ -11025,10 +11033,19 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree fntype,
 		      <= 8))
 		rs6000_returns_struct = true;
 	    }
-	  if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (return_mode))
-	    rs6000_passes_float = true;
-	  else if (ALTIVEC_OR_VSX_VECTOR_MODE (return_mode)
-		   || SPE_VECTOR_MODE (return_mode))
+	  if (SCALAR_FLOAT_MODE_P (return_mode))
+	    {
+	      rs6000_passes_float = true;
+	      if ((HAVE_LD_PPC_GNU_ATTR || TARGET_64BIT)
+		  && (FLOAT128_IBM_P (return_mode)
+		      || FLOAT128_IEEE_P (return_mode)
+		      || (return_type != NULL
+			  && (TYPE_MAIN_VARIANT (return_type)
+			      == long_double_type_node))))
+		rs6000_passes_long_double = true;
+	    }
+	  if (ALTIVEC_OR_VSX_VECTOR_MODE (return_mode)
+	      || SPE_VECTOR_MODE (return_mode))
 	    rs6000_passes_vector = true;
 	}
     }
@@ -11475,16 +11492,23 @@ rs6000_function_arg_advance_1 (CUMULATIVE_ARGS *cum, machine_mode mode,
     cum->nargs_prototype--;
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-  if (DEFAULT_ABI == ABI_V4
+  if (TARGET_ELF && (TARGET_64BIT || DEFAULT_ABI == ABI_V4)
       && cum->escapes)
     {
-      if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (mode))
-	rs6000_passes_float = true;
-      else if (named && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
-	rs6000_passes_vector = true;
-      else if (SPE_VECTOR_MODE (mode)
-	       && !cum->stdarg
-	       && cum->sysv_gregno <= GP_ARG_MAX_REG)
+      if (SCALAR_FLOAT_MODE_P (mode))
+	{
+	  rs6000_passes_float = true;
+	  if ((HAVE_LD_PPC_GNU_ATTR || TARGET_64BIT)
+	      && (FLOAT128_IBM_P (mode)
+		  || FLOAT128_IEEE_P (mode)
+		  || (type != NULL
+		      && TYPE_MAIN_VARIANT (type) == long_double_type_node)))
+	    rs6000_passes_long_double = true;
+	}
+      if ((named && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
+	  || (SPE_VECTOR_MODE (mode)
+	      && !cum->stdarg
+	      && cum->sysv_gregno <= GP_ARG_MAX_REG))
 	rs6000_passes_vector = true;
     }
 #endif
@@ -34292,13 +34316,21 @@ static void
 rs6000_elf_file_end (void)
 {
 #ifdef HAVE_AS_GNU_ATTRIBUTE
+  /* ??? The value emitted depends on options active at file end.
+     Assume anyone using #pragma or attributes that might change
+     options knows what they are doing.  */
+  if ((TARGET_64BIT || DEFAULT_ABI == ABI_V4)
+      && rs6000_passes_float)
+    fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n",
+	     ((TARGET_DF_FPR | TARGET_DF_SPE ? 1
+	       : TARGET_SF_FPR | TARGET_SF_SPE ? 3
+	       : 2)
+	      | (!rs6000_passes_long_double ? 0
+		 : !TARGET_LONG_DOUBLE_128 ? 2 * 4
+		 : TARGET_IEEEQUAD ? 3 * 4
+		 : 1 * 4)));
   if (TARGET_32BIT && DEFAULT_ABI == ABI_V4)
     {
-      if (rs6000_passes_float)
-	fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n",
-		 ((TARGET_HARD_FLOAT && TARGET_FPRS && TARGET_DOUBLE_FLOAT) ? 1 
-		  : (TARGET_HARD_FLOAT && TARGET_FPRS && TARGET_SINGLE_FLOAT) ? 3 
-		  : 2));
       if (rs6000_passes_vector)
 	fprintf (asm_out_file, "\t.gnu_attribute 8, %d\n",
 		 (TARGET_ALTIVEC_ABI ? 2
@@ -37085,6 +37117,9 @@ static struct rs6000_opt_var const rs6000_opt_vars[] =
   { "warn-cell-microcode",
     offsetof (struct gcc_options, x_rs6000_warn_cell_microcode),
     offsetof (struct cl_target_option, x_rs6000_warn_cell_microcode), },
+  { "gnu-attr",
+    offsetof (struct gcc_options, x_rs6000_gnu_attr),
+    offsetof (struct cl_target_option, x_rs6000_gnu_attr), },
 };
 
 /* Inner function to handle attribute((target("..."))) and #pragma GCC target
diff --git a/gcc/config/rs6000/sysv4.opt b/gcc/config/rs6000/sysv4.opt
index 581fcde..d5b27cc 100644
--- a/gcc/config/rs6000/sysv4.opt
+++ b/gcc/config/rs6000/sysv4.opt
@@ -155,3 +155,7 @@ Generate code to use a non-exec PLT and GOT.
 mbss-plt
 Target Report RejectNegative Var(secure_plt, 0) Save
 Generate code for old exec BSS PLT.
+
+mgnu-attr
+Target Report Var(rs6000_gnu_attr) Init(-1) Save
+Emit .gnu_attribute tags.
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 534f22e..94912a0 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5322,6 +5322,49 @@ if test "x$gcc_cv_ld_clearcap" = xyes; then
 fi
 AC_MSG_RESULT($gcc_cv_ld_clearcap)
 
+case "$target" in
+  powerpc*-*-*)
+    case "$target" in
+      *le-*-linux*)
+	emul_name="-melf32lppc"
+	;;
+      *)
+	emul_name="-melf32ppc"
+	;;
+    esac
+    AC_CACHE_CHECK(linker .gnu_attribute long double support,
+    gcc_cv_ld_ppc_attr,
+    [gcc_cv_ld_ppc_attr=no
+    if test x"$ld_is_gold" = xyes; then
+      gcc_cv_ld_ppc_attr=yes
+    elif test $in_tree_ld = yes ; then
+      if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28 -o "$gcc_cv_gld_major_version" -gt 2; then
+        gcc_cv_ld_ppc_attr=yes
+      fi
+    elif test x$gcc_cv_as != x -a x$gcc_cv_ld != x ; then
+      # check that merging the long double .gnu_attribute doesn't warn
+      cat > conftest1.s <<EOF
+	.gnu_attribute 4,1
+EOF
+      cat > conftest2.s <<EOF
+	.gnu_attribute 4,9
+EOF
+      if $gcc_cv_as -a32 -o conftest1.o conftest1.s > /dev/null 2>&1 \
+         && $gcc_cv_as -a32 -o conftest2.o conftest2.s > /dev/null 2>&1 \
+         && $gcc_cv_ld $emul_name -r -o conftest.o conftest1.o conftest2.o > /dev/null 2> conftest.err \
+	 && test ! -s conftest.err; then
+        gcc_cv_ld_ppc_attr=yes
+      fi
+      rm -f conftest.err conftest.o conftest1.o conftest2.o conftest1.s conftest2.s
+    fi
+    ])
+    if test x$gcc_cv_ld_ppc_attr = xyes; then
+      AC_DEFINE(HAVE_LD_PPC_GNU_ATTR, 1,
+    [Define if your PowerPC linker has .gnu_attribute long double support.])
+    fi
+    ;;
+esac
+
 case "$target:$tm_file" in
   powerpc64-*-freebsd* | powerpc64*-*-linux* | powerpc*-*-linux*rs6000/biarch64.h*)
   case "$target" in

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
  2016-09-28  1:20 [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Alan Modra
@ 2016-09-28  1:26 ` Alan Modra
  2016-09-28  2:49   ` Joseph Myers
  2016-09-28  9:04   ` Segher Boessenkool
  2016-09-28  2:00 ` [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Joseph Myers
  2016-09-28  8:48 ` Segher Boessenkool
  2 siblings, 2 replies; 10+ messages in thread
From: Alan Modra @ 2016-09-28  1:26 UTC (permalink / raw)
  To: gcc-patches, libstdc++; +Cc: Segher Boessenkool

compatibility-ldbl.o is compiled with -mlong-double-64.  When
long double .gnu_attribute tags are checked by the linker, it
complains about the mismatch between this file and others in
libstdc++.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux biarch.  OK to apply?

	* configure.ac (LONG_DOUBLE_COMPAT_FLAGS): New ACSUBST.
	* src/Makefile.am (compatibility-ldbl.o, compatibility-ldbl.lo):
	Use LONG_DOUBLE_COMPAT_FLAGS.
	* Makefile.in: Regenerate.
	* configure: Regenerate.
	* doc/Makefile.in: Regenerate.
	* include/Makefile.in: Regenerate.
	* libsupc++/Makefile.in: Regenerate.
	* po/Makefile.in: Regenerate.
	* python/Makefile.in: Regenerate.
	* src/Makefile.in: Regenerate.
	* src/c++11/Makefile.in: Regenerate.
	* src/c++98/Makefile.in: Regenerate.
	* src/filesystem/Makefile.in: Regenerate.
	* testsuite/Makefile.in: Regenerate.

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index baf605c..4a5de8c 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -375,6 +375,7 @@ GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI([yes])
 GLIBCXX_DEFAULT_ABI
 
 ac_ldbl_compat=no
+LONG_DOUBLE_COMPAT_FLAGS="-mlong-double-64"
 case "$target" in
   powerpc*-*-linux* | \
   sparc*-*-linux* | \
@@ -389,8 +390,13 @@ case "$target" in
     AC_DEFINE([_GLIBCXX_LONG_DOUBLE_COMPAT],1,
 	      [Define if compatibility should be provided for -mlong-double-64.])
     port_specific_symbol_files="\$(top_srcdir)/config/os/gnu-linux/ldbl-extra.ver"
+    case "$target" in
+      powerpc*-*-linux*)
+	LONG_DOUBLE_COMPAT_FLAGS="$LONG_DOUBLE_COMPAT_FLAGS -mno-gnu-attr" ;;
+    esac
   fi
 esac
+AC_SUBST(LONG_DOUBLE_COMPAT_FLAGS)
 GLIBCXX_CONDITIONAL(GLIBCXX_LDBL_COMPAT, test $ac_ldbl_compat = yes)
 
 # Check if assembler supports disabling hardware capability support.
diff --git a/libstdc++-v3/src/Makefile.am b/libstdc++-v3/src/Makefile.am
index 00a6168..d9830c2 100644
--- a/libstdc++-v3/src/Makefile.am
+++ b/libstdc++-v3/src/Makefile.am
@@ -109,9 +109,9 @@ libstdc___la_LINK = $(CXXLINK) $(libstdc___la_LDFLAGS)
 # pass -mlong-double-64.
 if GLIBCXX_LDBL_COMPAT
 compatibility-ldbl.lo: compatibility-ldbl.cc
-	$(LTCXXCOMPILE) -mlong-double-64 -c $<
+	$(LTCXXCOMPILE) $(LONG_DOUBLE_COMPAT_FLAGS) -c $<
 compatibility-ldbl.o: compatibility-ldbl.cc
-	$(CXXCOMPILE) -mlong-double-64 -c $<
+	$(CXXCOMPILE) $(LONG_DOUBLE_COMPAT_FLAGS) -c $<
 endif
 
 # Use special rules for C++11 files/objects.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
  2016-09-28  1:20 [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Alan Modra
  2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
@ 2016-09-28  2:00 ` Joseph Myers
  2016-09-28  4:38   ` Alan Modra
  2016-09-28  8:48 ` Segher Boessenkool
  2 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2016-09-28  2:00 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches, Segher Boessenkool

On Wed, 28 Sep 2016, Alan Modra wrote:

> I've also added a new option, default on, that disables output of
> .gnu_attribute tags.  That's needed because this patch alone
> introduces libstdc++ testsuite regressions, fixed by the next patch.

This option is missing documentation in invoke.texi.

> +mgnu-attr
> +Target Report Var(rs6000_gnu_attr) Init(-1) Save
> +Emit .gnu_attribute tags.

Why Init(-1)?  That's normally for cases where there is code that checks 
if it's still -1 (i.e. no option passed explicitly) and does something 
different in that case, but I don't see any such code in this patch.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
  2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
@ 2016-09-28  2:49   ` Joseph Myers
  2016-09-28  4:41     ` Alan Modra
  2016-09-28  9:04   ` Segher Boessenkool
  1 sibling, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2016-09-28  2:49 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches, libstdc++, Segher Boessenkool

On Wed, 28 Sep 2016, Alan Modra wrote:

> compatibility-ldbl.o is compiled with -mlong-double-64.  When
> long double .gnu_attribute tags are checked by the linker, it
> complains about the mismatch between this file and others in
> libstdc++.

Is that the only file in libstdc++ that involves long double in its 
interface at all, and so that gets such attributes?

I'd expect libraries such as libstdc++ and libgcc (generally, all compiler 
and libc libraries) to be set up in such a way that they will work with 
all long double choices in user code (via mangling and headers mapping 
access to long double library functions to the right versions for the 
chosen type) - and so need to be compiled without these attribute tags to 
avoid the linker complaining when someone links them with user code built 
with a non-default choice of long double.  Certainly for glibc I'd think 
using the option globally to build everything is the right choice (well, 
except for libnldbl.a, where -mlong-double-64 attributes are logically 
correct).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
  2016-09-28  2:00 ` [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Joseph Myers
@ 2016-09-28  4:38   ` Alan Modra
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Modra @ 2016-09-28  4:38 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, Segher Boessenkool

On Wed, Sep 28, 2016 at 01:20:22AM +0000, Joseph Myers wrote:
> On Wed, 28 Sep 2016, Alan Modra wrote:
> 
> > I've also added a new option, default on, that disables output of
> > .gnu_attribute tags.  That's needed because this patch alone
> > introduces libstdc++ testsuite regressions, fixed by the next patch.
> 
> This option is missing documentation in invoke.texi.

Oops, I'll add that.

> > +mgnu-attr
> > +Target Report Var(rs6000_gnu_attr) Init(-1) Save
> > +Emit .gnu_attribute tags.
> 
> Why Init(-1)?  That's normally for cases where there is code that checks 
> if it's still -1 (i.e. no option passed explicitly) and does something 
> different in that case, but I don't see any such code in this patch.

Yes, it can be Init(1).  I copied from other options in sysv4.opt.

Incidentally, in playing with #pragma GCC target "-mno-gnu-attr" I
find that it affects all functions in a file, rather than just
functions defined later in the file.  I'll look into that too.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
  2016-09-28  2:49   ` Joseph Myers
@ 2016-09-28  4:41     ` Alan Modra
  2016-09-28 17:34       ` Joseph Myers
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2016-09-28  4:41 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, libstdc++, Segher Boessenkool

On Wed, Sep 28, 2016 at 01:26:12AM +0000, Joseph Myers wrote:
> On Wed, 28 Sep 2016, Alan Modra wrote:
> 
> > compatibility-ldbl.o is compiled with -mlong-double-64.  When
> > long double .gnu_attribute tags are checked by the linker, it
> > complains about the mismatch between this file and others in
> > libstdc++.
> 
> Is that the only file in libstdc++ that involves long double in its 
> interface at all, and so that gets such attributes?

No, some 12 object files do, all marked with 128-bit IBM long double.
eg. c++locale.o, complex_io.o, hash_tr1.o.

> I'd expect libraries such as libstdc++ and libgcc (generally, all compiler 
> and libc libraries) to be set up in such a way that they will work with 
> all long double choices in user code (via mangling and headers mapping 
> access to long double library functions to the right versions for the 
> chosen type) - and so need to be compiled without these attribute tags to 
> avoid the linker complaining when someone links them with user code built 
> with a non-default choice of long double.  Certainly for glibc I'd think 
> using the option globally to build everything is the right choice (well, 
> except for libnldbl.a, where -mlong-double-64 attributes are logically 
> correct).

Yes, and this is why the linker only warns rather than errors on
mismatching .gnu.attributes tags.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
  2016-09-28  1:20 [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Alan Modra
  2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
  2016-09-28  2:00 ` [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Joseph Myers
@ 2016-09-28  8:48 ` Segher Boessenkool
  2016-09-29  0:57   ` Alan Modra
  2 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2016-09-28  8:48 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches

Hi Alan,

On Wed, Sep 28, 2016 at 10:41:45AM +0930, Alan Modra wrote:
> Extend this attribute to cover long double ABIs, for 64-bit too.  The
> idea is that the linker should warn if you are linking object files
> with incompatible ABIs, for example IEEE128 long double with IBM long
> double.  It's only a warning, and doesn't catch everything.  In
> particular, global data mismatches.  ie. initialised global variables
> in one format can be used by code that expects a different format
> without warning.  Also, a function returning "sizeof (long double)"
> or similar won't cause an object file to be tagged.  Oh, and you need
> a new linker to see long double warnings.
> 
> The patch also corrects an error that crept in to code setting
> rs6000_passes_float.  See the added comment.  Passing IEEE128 values
> in vsx regs ought to set both Tag_GNU_Power_ABI_FP and
> Tag_GNU_Power_ABI_Vector.
> 
> I've also added a new option, default on, that disables output of
> .gnu_attribute tags.  That's needed because this patch alone
> introduces libstdc++ testsuite regressions, fixed by the next patch.

>  #ifdef HAVE_AS_GNU_ATTRIBUTE
> +  /* ??? The value emitted depends on options active at file end.
> +     Assume anyone using #pragma or attributes that might change
> +     options knows what they are doing.  */
> +  if ((TARGET_64BIT || DEFAULT_ABI == ABI_V4)
> +      && rs6000_passes_float)
> +    fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n",
> +	     ((TARGET_DF_FPR | TARGET_DF_SPE ? 1
> +	       : TARGET_SF_FPR | TARGET_SF_SPE ? 3
> +	       : 2)
> +	      | (!rs6000_passes_long_double ? 0
> +		 : !TARGET_LONG_DOUBLE_128 ? 2 * 4
> +		 : TARGET_IEEEQUAD ? 3 * 4
> +		 : 1 * 4)));

This will become much more readable if you expand it to a few statements,
using a temp var perhaps.

> @@ -37085,6 +37117,9 @@ static struct rs6000_opt_var const rs6000_opt_vars[] =
>    { "warn-cell-microcode",
>      offsetof (struct gcc_options, x_rs6000_warn_cell_microcode),
>      offsetof (struct cl_target_option, x_rs6000_warn_cell_microcode), },
> +  { "gnu-attr",
> +    offsetof (struct gcc_options, x_rs6000_gnu_attr),
> +    offsetof (struct cl_target_option, x_rs6000_gnu_attr), },
>  };

Please spell out "gnu-attribute" for the option name.

> +      if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28 -o "$gcc_cv_gld_major_version" -gt 2; then

That line is a little long.

> +      AC_DEFINE(HAVE_LD_PPC_GNU_ATTR, 1,
> +    [Define if your PowerPC linker has .gnu_attribute long double support.])

"LD" didn't make me think "long double" ;-)  Maybe use a better name?

Okay for trunk with those thing fixed, and Joseph's comments taken care
of of course.

Thanks,


Segher

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
  2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
  2016-09-28  2:49   ` Joseph Myers
@ 2016-09-28  9:04   ` Segher Boessenkool
  1 sibling, 0 replies; 10+ messages in thread
From: Segher Boessenkool @ 2016-09-28  9:04 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches, libstdc++

On Wed, Sep 28, 2016 at 10:43:38AM +0930, Alan Modra wrote:
> compatibility-ldbl.o is compiled with -mlong-double-64.  When
> long double .gnu_attribute tags are checked by the linker, it
> complains about the mismatch between this file and others in
> libstdc++.
> 
> Bootstrapped and regression tested powerpc64le-linux and
> powerpc64-linux biarch.  OK to apply?

Okay with the option name fixed.  Thanks,


Segher

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
  2016-09-28  4:41     ` Alan Modra
@ 2016-09-28 17:34       ` Joseph Myers
  0 siblings, 0 replies; 10+ messages in thread
From: Joseph Myers @ 2016-09-28 17:34 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches, libstdc++, Segher Boessenkool

On Wed, 28 Sep 2016, Alan Modra wrote:

> > I'd expect libraries such as libstdc++ and libgcc (generally, all compiler 
> > and libc libraries) to be set up in such a way that they will work with 
> > all long double choices in user code (via mangling and headers mapping 
> > access to long double library functions to the right versions for the 
> > chosen type) - and so need to be compiled without these attribute tags to 
> > avoid the linker complaining when someone links them with user code built 
> > with a non-default choice of long double.  Certainly for glibc I'd think 
> > using the option globally to build everything is the right choice (well, 
> > except for libnldbl.a, where -mlong-double-64 attributes are logically 
> > correct).
> 
> Yes, and this is why the linker only warns rather than errors on
> mismatching .gnu.attributes tags.

But for a library that is aware of long double variants, it shouldn't even 
warn.  And given that we don't build multiple copies of GCC's libraries, 
they should be aware of the variants (via mangling them differently, 
ensuring versions of the relevant functions for each long double type are 
present, etc.) and so using them should not result in warnings.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
  2016-09-28  8:48 ` Segher Boessenkool
@ 2016-09-29  0:57   ` Alan Modra
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Modra @ 2016-09-29  0:57 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

Committed as svn r240601 and patch 2/2 as r240602.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-09-29  0:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-28  1:20 [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Alan Modra
2016-09-28  1:26 ` [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o Alan Modra
2016-09-28  2:49   ` Joseph Myers
2016-09-28  4:41     ` Alan Modra
2016-09-28 17:34       ` Joseph Myers
2016-09-28  9:04   ` Segher Boessenkool
2016-09-28  2:00 ` [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP Joseph Myers
2016-09-28  4:38   ` Alan Modra
2016-09-28  8:48 ` Segher Boessenkool
2016-09-29  0:57   ` Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).