diff mbox

[1/2,RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

Message ID 20160928011145.GF3336@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Sept. 28, 2016, 1:11 a.m. UTC 
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.

Comments

Joseph Myers Sept. 28, 2016, 1:20 a.m. UTC | #1 
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.
Alan Modra Sept. 28, 2016, 2 a.m. UTC | #2 
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.
Segher Boessenkool Sept. 28, 2016, 8:36 a.m. UTC | #3 
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
Alan Modra Sept. 29, 2016, 12:32 a.m. UTC | #4 
Committed as svn r240601 and patch 2/2 as r240602.
diff mbox

Patch

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