diff mbox

Remove -fshort-double (PR60410)

Message ID 56B4F87A.7060204@redhat.com
State New
Headers show

Commit Message

Bernd Schmidt Feb. 5, 2016, 7:31 p.m. UTC 
This patch fixes PR60410 by removing -fshort-double. Nick earlier 
propsed a fix for the crash, but Richard B suggested removing the option 
entirely, and I'd agree with that. It's a pointless ABI-changing option 
on most targets, and if a port really needs it, it should be a -m option 
that tweaks DOUBLE_TYPE_SIZE.

It turns out that there is still a mips config that enables it for a set 
of multilibs. As mentioned here:
   https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02140.html
I've not managed to make that config build, it fails configuring libgcc. 
A mips maintainer would have to speak up as to whether that config is 
useful at all or not.

In any case, the patch below was bootstrapped and tested on 
x86_64-linux. Ok?


Bernd

Comments

Jeff Law Feb. 5, 2016, 8:06 p.m. UTC | #1 
On 02/05/2016 12:31 PM, Bernd Schmidt wrote:
> This patch fixes PR60410 by removing -fshort-double. Nick earlier
> propsed a fix for the crash, but Richard B suggested removing the option
> entirely, and I'd agree with that. It's a pointless ABI-changing option
> on most targets, and if a port really needs it, it should be a -m option
> that tweaks DOUBLE_TYPE_SIZE.
>
> It turns out that there is still a mips config that enables it for a set
> of multilibs. As mentioned here:
>    https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02140.html
> I've not managed to make that config build, it fails configuring libgcc.
> A mips maintainer would have to speak up as to whether that config is
> useful at all or not.
>
> In any case, the patch below was bootstrapped and tested on
> x86_64-linux. Ok?
Shouldn't c-opt.def be changed to note the option is deprecated and 
ignored rather than totally removing it?

OK with that change.

Jeff
Matthew Fortune Feb. 6, 2016, 9 a.m. UTC | #2 
Jeff Law <law@redhat.com> writes:
> On 02/05/2016 12:31 PM, Bernd Schmidt wrote:

> > This patch fixes PR60410 by removing -fshort-double. Nick earlier

> > propsed a fix for the crash, but Richard B suggested removing the option

> > entirely, and I'd agree with that. It's a pointless ABI-changing option

> > on most targets, and if a port really needs it, it should be a -m option

> > that tweaks DOUBLE_TYPE_SIZE.

> >

> > It turns out that there is still a mips config that enables it for a set

> > of multilibs. As mentioned here:

> >    https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02140.html

> > I've not managed to make that config build, it fails configuring libgcc.

> > A mips maintainer would have to speak up as to whether that config is

> > useful at all or not.


Sorry for being slow to reply.

This configuration was added to MIPS as one way to make efficient use of
a single precision only FPU. The intention was to avoid promotion to
doubles where the calling convention would normally require it. The
prime example being floating point varargs arguments. There are however
a huge amount of failures in this mode when testing but it was good enough
for some users. Given there is an alternative then I have no objection
to removing it. It will be a couple of days before I have chance to
implement a -m option and update the mips-img-elf multilib.

Thanks,
Matthew
Joseph Myers Feb. 7, 2016, 2:30 p.m. UTC | #3 
On Fri, 5 Feb 2016, Jeff Law wrote:

> Shouldn't c-opt.def be changed to note the option is deprecated and ignored
> rather than totally removing it?

No.  That's appropriate with optimization options which can safely be 
ignored, not with options such as this that have an incompatible effect on 
ABI or API.
Jeff Law Feb. 8, 2016, 2:48 a.m. UTC | #4 
On 02/07/2016 07:30 AM, Joseph Myers wrote:
> On Fri, 5 Feb 2016, Jeff Law wrote:
>
>> Shouldn't c-opt.def be changed to note the option is deprecated and ignored
>> rather than totally removing it?
>
> No.  That's appropriate with optimization options which can safely be
> ignored, not with options such as this that have an incompatible effect on
> ABI or API.
Good point.  In that case I think the patch is fine as-is.

jeff
diff mbox

Patch

	PR target/60410
	* tree.c (build_common_tree_nodes): Remove short_double argument.
	All callers changed.
	* tree.h (build_common_tree_nodes): Adjust declaration.
	* doc/invoke.texi (-fshort-double): Remove documentation.
	* config/mips/t-img-elf (MULTILIB_OPTIONS, MULTILIB_DIRNAMES,
	MULTILIB_EXCEPTIONS): Remove -fshort-double variant.
	* lto-wrapper.c (merge_and_complain, append_compiler_options,
	append_linker_options): Don't handle OPT_fshort_double.
	
c-family/
	PR target/60410
	* c.opt (fshort-double): Remove.

testsuite/
	PR target/60410
	* gcc.dg/lto/pr55113_0.c: Remove test.

diff --git a/gcc/ada/gcc-interface/misc.c b/gcc/ada/gcc-interface/misc.c
index 992ac0a..75e467b 100644
--- a/gcc/ada/gcc-interface/misc.c
+++ b/gcc/ada/gcc-interface/misc.c
@@ -355,7 +355,7 @@  gnat_init (void)
 {
   /* Do little here, most of the standard declarations are set up after the
      front-end has been run.  Use the same `char' as C for Interfaces.C.  */
-  build_common_tree_nodes (flag_signed_char, false);
+  build_common_tree_nodes (flag_signed_char);
 
   /* In Ada, we use an unsigned 8-bit type for the default boolean type.  */
   boolean_type_node = make_unsigned_type (8);
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 378afae..3d84316 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5237,7 +5237,7 @@  c_common_nodes_and_builtins (void)
   tree va_list_arg_type_node;
   int i;
 
-  build_common_tree_nodes (flag_signed_char, flag_short_double);
+  build_common_tree_nodes (flag_signed_char);
 
   /* Define `int' and `char' first so that dbx will output them first.  */
   record_builtin_type (RID_INT, NULL, integer_type_node);
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index f243744..24858cd 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1435,10 +1435,6 @@  frtti
 C++ ObjC++ Optimization Var(flag_rtti) Init(1)
 Generate run time type descriptor information.
 
-fshort-double
-C ObjC C++ ObjC++ LTO Optimization Var(flag_short_double)
-Use the same size for double as for float.
-
 fshort-enums
 C ObjC C++ ObjC++ LTO Optimization Var(flag_short_enums)
 Use the narrowest integer type possible for enumeration types.
diff --git a/gcc/config/mips/t-img-elf b/gcc/config/mips/t-img-elf
index eca0a2e..8c22853 100644
--- a/gcc/config/mips/t-img-elf
+++ b/gcc/config/mips/t-img-elf
@@ -20,19 +20,14 @@ 
 # A multilib for mips32r6+LE
 # A multilib for mips64r6
 # A multilib for mips64r6+LE
-# A multilib for mips32r6+LE+singlefloat+shortdouble
 
-MULTILIB_OPTIONS = mips64r6 mabi=64 EL msoft-float/msingle-float fshort-double
-MULTILIB_DIRNAMES = mips64r6 64 el sof sgl short
+MULTILIB_OPTIONS = mips64r6 mabi=64 EL msoft-float/msingle-float
+MULTILIB_DIRNAMES = mips64r6 64 el sof sgl
 MULTILIB_MATCHES = EL=mel EB=meb
 
 # Don't build 64r6 with single-float
 MULTILIB_EXCEPTIONS += mips64r6/*msingle-float*
-MULTILIB_EXCEPTIONS += mips64r6/*fshort-double*
 
 MULTILIB_EXCEPTIONS += mabi=64*
 MULTILIB_EXCEPTIONS += msingle-float*
 MULTILIB_EXCEPTIONS += *msingle-float
-MULTILIB_EXCEPTIONS += fshort-double
-MULTILIB_EXCEPTIONS += EL/fshort-double
-MULTILIB_EXCEPTIONS += *msoft-float/fshort-double
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index fcc404e..ad536ba 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -488,8 +488,7 @@  Objective-C and Objective-C++ Dialects}.
 -fpcc-struct-return  -fpic  -fPIC -fpie -fPIE -fno-plt @gol
 -fno-jump-tables @gol
 -frecord-gcc-switches @gol
--freg-struct-return  -fshort-enums @gol
--fshort-double  -fshort-wchar @gol
+-freg-struct-return  -fshort-enums  -fshort-wchar @gol
 -fverbose-asm  -fpack-struct[=@var{n}]  @gol
 -fleading-underscore  -ftls-model=@var{model} @gol
 -fstack-reuse=@var{reuse_level} @gol
@@ -11090,14 +11089,6 @@  is equivalent to the smallest integer type that has enough room.
 code that is not binary compatible with code generated without that switch.
 Use it to conform to a non-default application binary interface.
 
-@item -fshort-double
-@opindex fshort-double
-Use the same size for @code{double} as for @code{float}.
-
-@strong{Warning:} the @option{-fshort-double} switch causes GCC to generate
-code that is not binary compatible with code generated without that switch.
-Use it to conform to a non-default application binary interface.
-
 @item -fshort-wchar
 @opindex fshort-wchar
 Override the underlying type for @code{wchar_t} to be @code{short
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 9c3a311..b89a291 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -491,9 +491,8 @@  gfc_init_decl_processing (void)
   global_binding_level = current_binding_level;
 
   /* Build common tree nodes. char_type_node is unsigned because we
-     only use it for actual characters, not for INTEGER(1). Also, we
-     want double_type_node to actually have double precision.  */
-  build_common_tree_nodes (false, false);
+     only use it for actual characters, not for INTEGER(1).  */
+  build_common_tree_nodes (false);
 
   void_list_node = build_tree_list (NULL_TREE, void_type_node);
 
diff --git a/gcc/go/go-lang.c b/gcc/go/go-lang.c
index 9f21424..9c95c8e 100644
--- a/gcc/go/go-lang.c
+++ b/gcc/go/go-lang.c
@@ -89,7 +89,7 @@  static const char *go_relative_import_path = NULL;
 static bool
 go_langhook_init (void)
 {
-  build_common_tree_nodes (false, false);
+  build_common_tree_nodes (false);
 
   /* I don't know why this has to be done explicitly.  */
   void_list_node = build_tree_list (NULL_TREE, void_type_node);
diff --git a/gcc/java/decl.c b/gcc/java/decl.c
index 472b199..93304da 100644
--- a/gcc/java/decl.c
+++ b/gcc/java/decl.c
@@ -565,7 +565,7 @@  java_init_decl_processing (void)
   global_binding_level = current_binding_level;
 
   /* Build common tree nodes, Java has an unsigned char.  */
-  build_common_tree_nodes (false, false);
+  build_common_tree_nodes (false);
 
   /* ???  Now we continue and override some of the built types again
      with Java specific types.  As the above generated types are
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index ced6f2f..6a337a0 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -308,7 +308,6 @@  merge_and_complain (struct cl_decoded_option **decoded_options,
 
 	case OPT_freg_struct_return:
 	case OPT_fpcc_struct_return:
-	case OPT_fshort_double:
 	  for (j = 0; j < *decoded_options_count; ++j)
 	    if ((*decoded_options)[j].opt_index == foption->opt_index)
 	      break;
@@ -511,7 +510,6 @@  append_compiler_options (obstack *argv_obstack, struct cl_decoded_option *opts,
 	case OPT_fgnu_tm:
 	case OPT_freg_struct_return:
 	case OPT_fpcc_struct_return:
-	case OPT_fshort_double:
 	case OPT_ffp_contract_:
 	case OPT_fmath_errno:
 	case OPT_fsigned_zeros:
@@ -569,7 +567,6 @@  append_linker_options (obstack *argv_obstack, struct cl_decoded_option *opts,
 
 	case OPT_freg_struct_return:
 	case OPT_fpcc_struct_return:
-	case OPT_fshort_double:
 	  /* Ignore these, they are determined by the input files.
 	     ???  We fail to diagnose a possible mismatch here.  */
 	  continue;
diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c
index ec4db44..691e9e2 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -1246,7 +1246,7 @@  lto_init (void)
   flag_generate_lto = (flag_wpa != NULL);
 
   /* Create the basic integer types.  */
-  build_common_tree_nodes (flag_signed_char, flag_short_double);
+  build_common_tree_nodes (flag_signed_char);
 
   /* The global tree for the main identifier is filled in by
      language-specific front-end initialization that is not run in the
diff --git a/gcc/testsuite/gcc.dg/lto/pr55113_0.c b/gcc/testsuite/gcc.dg/lto/pr55113_0.c
deleted file mode 100644
index 59e944c..0000000
--- a/gcc/testsuite/gcc.dg/lto/pr55113_0.c
+++ /dev/null
@@ -1,13 +0,0 @@ 
-/* PR 55113 */
-/* { dg-lto-do link } */
-/* { dg-lto-options { { -flto -fshort-double -O0 } } }*/
-/* { dg-skip-if "PR60410" { i?86-*-* x86_64-*-* } } */
-
-int 
-main(void)
-{
-  float a = 1.0;
-  float b = 2.0;
-  double f = a + b * 1e-12;
-  return (int)f - 1;
-}
diff --git a/gcc/tree.c b/gcc/tree.c
index fa7646b..ac3efd3 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10036,12 +10036,10 @@  build_atomic_base (tree type, unsigned int align)
 }
 
 /* Create nodes for all integer types (and error_mark_node) using the sizes
-   of C datatypes.  SIGNED_CHAR specifies whether char is signed,
-   SHORT_DOUBLE specifies whether double should be of the same precision
-   as float.  */
+   of C datatypes.  SIGNED_CHAR specifies whether char is signed.  */
 
 void
-build_common_tree_nodes (bool signed_char, bool short_double)
+build_common_tree_nodes (bool signed_char)
 {
   int i;
 
@@ -10202,10 +10200,7 @@  build_common_tree_nodes (bool signed_char, bool short_double)
   layout_type (float_type_node);
 
   double_type_node = make_node (REAL_TYPE);
-  if (short_double)
-    TYPE_PRECISION (double_type_node) = FLOAT_TYPE_SIZE;
-  else
-    TYPE_PRECISION (double_type_node) = DOUBLE_TYPE_SIZE;
+  TYPE_PRECISION (double_type_node) = DOUBLE_TYPE_SIZE;
   layout_type (double_type_node);
 
   long_double_type_node = make_node (REAL_TYPE);
diff --git a/gcc/tree.h b/gcc/tree.h
index 191f21b..544a6a1 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4763,7 +4763,7 @@  extern tree reconstruct_complex_type (tree, tree);
 extern int real_onep (const_tree);
 extern int real_minus_onep (const_tree);
 extern void init_ttree (void);
-extern void build_common_tree_nodes (bool, bool);
+extern void build_common_tree_nodes (bool);
 extern void build_common_builtin_nodes (void);
 extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT, int);
 extern tree build_nonstandard_boolean_type (unsigned HOST_WIDE_INT);