Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-08 Thread Paulo Matos 

On 07/03/14 09:03, Richard Biener wrote:


Btw, can you check the attached as well?  It makes sure all TUs
have -fshort-double set consistently and it automatically enables
it at link-time, not allowing to override the setting.

If it works for you please check it in, too.  (I can't really test it
because -fshort-double brokeness on x86_64).



I have tested it and everything looks fine. I have now committed all the 
code and testcase.  Hopefully it's now sorted.


Thanks for your help,

Paulo Matos


Thanks,
Richard.




--
PMatos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-07 Thread Richard Biener 
On Fri, Mar 7, 2014 at 12:29 AM, Paulo Matos pa...@matos-sorge.com wrote:
 On 06/03/14 11:19, Richard Biener wrote:

 On Wed, Mar 5, 2014 at 12:55 PM, Paulo Matos pa...@matos-sorge.com
 wrote:

 On 05/03/2014 11:51, Richard Biener wrote:


 On Wed, Mar 5, 2014 at 12:43 PM, Dominique Dhumieres
 domi...@lps.ens.fr
 wrote:



 Revision 208312 causes

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60427



 Uhm.  pointer comparison against double_type_node ...

 I'd say we want to revert the patch.  Paulo, please do that.  Let's
 do the alternate approach of marking -fshort-double eligible for LTO
 as well and handle it there properly.


 Sure, I will prepare a new patch and post it for approval by the end of
 the
 day.

 Apologies for the regression.


 I have reverted the patch for now.

 Richard.



 Please find new patch attached. I have enabled LTO for short-double and
 passed flag_short_double to build_common_tree_nodes.

 I have tested this for C on powerpc-eabipse (target for which we are fixing
 pr55113), and C/Fortran on a x86_64. Saw no regressions.

 OK to commit?

Ok.

Thanks,
Richard.

 gcc/c-family/
 2014-03-06  Paulo Matos  pa...@matos-sorge.com

 * c.opt: Enable LTO FE for fshort-double.

 gcc/lto/
 2014-03-06  Paulo Matos  pa...@matos-sorge.com

 * lto-lang.c (lto_init): Pass flag_short_double to
 build_common_tree_nodes.

 gcc/testsuite/
 2014-03-06  Paulo Matos  pa...@matos-sorge.com

 * gcc.dg/lto/pr55113_0.c: New testcase.


 --
 Paulo Matos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-06 Thread Richard Biener 
On Wed, Mar 5, 2014 at 12:55 PM, Paulo Matos pa...@matos-sorge.com wrote:
 On 05/03/2014 11:51, Richard Biener wrote:

 On Wed, Mar 5, 2014 at 12:43 PM, Dominique Dhumieres domi...@lps.ens.fr
 wrote:


 Revision 208312 causes

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60427


 Uhm.  pointer comparison against double_type_node ...

 I'd say we want to revert the patch.  Paulo, please do that.  Let's
 do the alternate approach of marking -fshort-double eligible for LTO
 as well and handle it there properly.


 Sure, I will prepare a new patch and post it for approval by the end of the
 day.

 Apologies for the regression.

I have reverted the patch for now.

Richard.

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-06 Thread Paulo Matos 

On 06/03/2014 11:19, Richard Biener wrote:


I have reverted the patch for now.

Richard.


That's fine Richard, thanks. I got stuck with another issue in the 
meantime but I will look at it again very soon.


--
Paulo Matos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-05 Thread Dominique Dhumieres 

Revision 208312 causes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60427

Also compiling the test gcc.dg/lto/pr55113 with -m32 gives an ICE

FAIL: gcc.dg/lto/pr55113 c_lto_pr55113_0.o assemble,  -flto -fshort-double -O0  
(internal compiler error)

internal compiler error: in layout_type, at stor-layout.c:2116

TIA

Dominique

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-05 Thread Richard Biener 
On Wed, Mar 5, 2014 at 12:43 PM, Dominique Dhumieres domi...@lps.ens.fr wrote:

 Revision 208312 causes

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60427

Uhm.  pointer comparison against double_type_node ...

I'd say we want to revert the patch.  Paulo, please do that.  Let's
do the alternate approach of marking -fshort-double eligible for LTO
as well and handle it there properly.

Thanks,
Richard.

 Also compiling the test gcc.dg/lto/pr55113 with -m32 gives an ICE

 FAIL: gcc.dg/lto/pr55113 c_lto_pr55113_0.o assemble,  -flto -fshort-double 
 -O0  (internal compiler error)

 internal compiler error: in layout_type, at stor-layout.c:2116

 TIA

 Dominique

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-05 Thread Paulo Matos 

On 05/03/2014 11:51, Richard Biener wrote:
On Wed, Mar 5, 2014 at 12:43 PM, Dominique Dhumieres 
domi...@lps.ens.fr wrote:


Revision 208312 causes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60427


Uhm.  pointer comparison against double_type_node ...

I'd say we want to revert the patch.  Paulo, please do that.  Let's
do the alternate approach of marking -fshort-double eligible for LTO
as well and handle it there properly.



Sure, I will prepare a new patch and post it for approval by the end of 
the day.


Apologies for the regression.
--
Paulo Matos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-04 Thread Paulo J. Matos 

On 03/03/14 09:56, Richard Biener wrote:

Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt  (revision 208249)
+++ gcc/c-family/c.opt  (working copy)
@@ -1141,7 +1141,7 @@ C++ ObjC++ Optimization Var(flag_rtti) I
  Generate run time type descriptor information

  fshort-double
-C ObjC C++ ObjC++ Optimization Var(flag_short_double)
+C ObjC C++ ObjC++ LTO Optimization Var(flag_short_double)
  Use the same size for double as for float

This hunk isn't needed.

Index: gcc/testsuite/gcc.target/powerpc/pr55113.c
===
--- gcc/testsuite/gcc.target/powerpc/pr55113.c  (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr55113.c  (working copy)
@@ -0,0 +1,11 @@
+#include stdio.h
+
+int main()
+{
+   static float f;
+   float a = 1.0;
+   float b = 2.0;
+   f = a + b * 1e-12;
+   printf(%f\n, f);
+   return 0;
+}

that doesn't seem to be run with -flto nor -fshort-double.  The proper
place for a testcase is gcc.dg/lto/ with sth like

{ dg-lto-do link }
{ dg-lto-options { { -O2 -fshort-double -flto } } }

and naming the testcase pr55113_0.c.  Your testcase doens't use
doubles at all ... (well, ok, a + b * 1e-12 uses them implicitely, but
for that we have fsingle-precision-constant)

Richard.


--
PMatos




Thanks for all your comments. I have updated the patch to reflect your 
comments and concerns.


2014-03-04  Paulo Matos  pa...@matos-sorge.com

* tree-streamer.c (record_common_node): Assert we don't record
nodes with type double.
(preload_common_node): Skip type double, complex double and
double pointer since it is now frontend dependent due to
fshort-double option.

2014-03-04  Paulo Matos  pa...@matos-sorge.com

* gcc.dg/lto/pr55113_0.c: New testcase.

There is an issue opened by this bug. i386 fails the test but not due to 
lto. It fails due to short-double:
$ top-trunk/toolchain/install-native/bin/gcc -O2 -o test pr55113_0.c 
-fshort-double

built-in: internal compiler error: in layout_type, at stor-layout.c:2116
0xb36233 layout_type(tree_node*)
../../../src/gcc/gcc/stor-layout.c:2115
0xdf8998 make_vector_type
../../../src/gcc/gcc/tree.c:9431
0xdfbb87 build_vector_type_for_mode(tree_node*, machine_mode)
../../../src/gcc/gcc/tree.c:10205
0xe8dd57 ix86_get_builtin_type
../../../src/gcc/gcc/config/i386/i386.c:27021
0xe8de95 ix86_get_builtin_func_type
../../../src/gcc/gcc/config/i386/i386.c:27071
0xe8dfa1 def_builtin
../../../src/gcc/gcc/config/i386/i386.c:28787
0xe8e662 ix86_init_mmx_sse_builtins
../../../src/gcc/gcc/config/i386/i386.c:30724
0xe92e8f ix86_init_builtins
../../../src/gcc/gcc/config/i386/i386.c:32677
0x62c692 c_define_builtins
../../../src/gcc/gcc/c-family/c-common.c:5268
0x62e09c c_common_nodes_and_builtins()
../../../src/gcc/gcc/c-family/c-common.c:5712
0x57b859 c_init_decl_processing()
../../../src/gcc/gcc/c/c-decl.c:3550
0x5d260f c_objc_common_init()
../../../src/gcc/gcc/c/c-objc-common.c:63
0xb4333c lang_dependent_init
../../../src/gcc/gcc/toplev.c:1712
0xb4374a do_compile
../../../src/gcc/gcc/toplev.c:1900
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See http://gcc.gnu.org/bugs.html for instructions.


I think we can close PR55113 and open a new bug for i386 and 
fshort-double usage if there isn't one open already?


OK to commit and close bug?
--
PMatos
Index: gcc/tree-streamer.c
===
--- gcc/tree-streamer.c	(revision 208249)
+++ gcc/tree-streamer.c	(working copy)
@@ -264,7 +264,8 @@ record_common_node (struct streamer_tree
 
   gcc_checking_assert (node != boolean_type_node
 		node != boolean_true_node
-		node != boolean_false_node);
+		node != boolean_false_node
+		node != double_type_node);
 
   /* We have to make sure to fill exactly the same number of
  elements for all frontends.  That can include NULL trees.
@@ -315,10 +316,14 @@ preload_common_nodes (struct streamer_tr
 record_common_node (cache, sizetype_tab[i]);
 
   for (i = 0; i  TI_MAX; i++)
-/* Skip boolean type and constants, they are frontend dependent.  */
+/* Skip boolean type and constants. They are frontend dependent.
+   Skip double type, frontend dependent due to -fshort-double.  */
 if (i != TI_BOOLEAN_TYPE
 	 i != TI_BOOLEAN_FALSE
-	 i != TI_BOOLEAN_TRUE)
+	 i != TI_BOOLEAN_TRUE
+	 i != TI_DOUBLE_TYPE
+	 i != TI_COMPLEX_DOUBLE_TYPE
+	 i != TI_DOUBLE_PTR_TYPE)
   record_common_node (cache, global_trees[i]);
 }
 
Index: gcc/testsuite/gcc.dg/lto/pr55113_0.c
===
--- gcc/testsuite/gcc.dg/lto/pr55113_0.c	(revision 0)
+++

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-04 Thread Richard Biener 
On Tue, Mar 4, 2014 at 12:02 PM, Paulo J. Matos pa...@matos-sorge.com wrote:
 On 03/03/14 09:56, Richard Biener wrote:

 Index: gcc/c-family/c.opt
 ===
 --- gcc/c-family/c.opt  (revision 208249)
 +++ gcc/c-family/c.opt  (working copy)
 @@ -1141,7 +1141,7 @@ C++ ObjC++ Optimization Var(flag_rtti) I
   Generate run time type descriptor information

   fshort-double
 -C ObjC C++ ObjC++ Optimization Var(flag_short_double)
 +C ObjC C++ ObjC++ LTO Optimization Var(flag_short_double)
   Use the same size for double as for float

 This hunk isn't needed.

 Index: gcc/testsuite/gcc.target/powerpc/pr55113.c
 ===
 --- gcc/testsuite/gcc.target/powerpc/pr55113.c  (revision 0)
 +++ gcc/testsuite/gcc.target/powerpc/pr55113.c  (working copy)
 @@ -0,0 +1,11 @@
 +#include stdio.h
 +
 +int main()
 +{
 +   static float f;
 +   float a = 1.0;
 +   float b = 2.0;
 +   f = a + b * 1e-12;
 +   printf(%f\n, f);
 +   return 0;
 +}

 that doesn't seem to be run with -flto nor -fshort-double.  The proper
 place for a testcase is gcc.dg/lto/ with sth like

 { dg-lto-do link }
 { dg-lto-options { { -O2 -fshort-double -flto } } }

 and naming the testcase pr55113_0.c.  Your testcase doens't use
 doubles at all ... (well, ok, a + b * 1e-12 uses them implicitely, but
 for that we have fsingle-precision-constant)

 Richard.

 --
 PMatos



 Thanks for all your comments. I have updated the patch to reflect your
 comments and concerns.

 2014-03-04  Paulo Matos  pa...@matos-sorge.com


 * tree-streamer.c (record_common_node): Assert we don't record
 nodes with type double.
 (preload_common_node): Skip type double, complex double and
 double pointer since it is now frontend dependent due to
 fshort-double option.

 2014-03-04  Paulo Matos  pa...@matos-sorge.com

 * gcc.dg/lto/pr55113_0.c: New testcase.

 There is an issue opened by this bug. i386 fails the test but not due to
 lto. It fails due to short-double:
 $ top-trunk/toolchain/install-native/bin/gcc -O2 -o test pr55113_0.c
 -fshort-double
 built-in: internal compiler error: in layout_type, at stor-layout.c:2116
 0xb36233 layout_type(tree_node*)
 ../../../src/gcc/gcc/stor-layout.c:2115
 0xdf8998 make_vector_type
 ../../../src/gcc/gcc/tree.c:9431
 0xdfbb87 build_vector_type_for_mode(tree_node*, machine_mode)
 ../../../src/gcc/gcc/tree.c:10205
 0xe8dd57 ix86_get_builtin_type
 ../../../src/gcc/gcc/config/i386/i386.c:27021
 0xe8de95 ix86_get_builtin_func_type
 ../../../src/gcc/gcc/config/i386/i386.c:27071
 0xe8dfa1 def_builtin
 ../../../src/gcc/gcc/config/i386/i386.c:28787
 0xe8e662 ix86_init_mmx_sse_builtins
 ../../../src/gcc/gcc/config/i386/i386.c:30724
 0xe92e8f ix86_init_builtins
 ../../../src/gcc/gcc/config/i386/i386.c:32677
 0x62c692 c_define_builtins
 ../../../src/gcc/gcc/c-family/c-common.c:5268
 0x62e09c c_common_nodes_and_builtins()
 ../../../src/gcc/gcc/c-family/c-common.c:5712
 0x57b859 c_init_decl_processing()
 ../../../src/gcc/gcc/c/c-decl.c:3550
 0x5d260f c_objc_common_init()
 ../../../src/gcc/gcc/c/c-objc-common.c:63
 0xb4333c lang_dependent_init
 ../../../src/gcc/gcc/toplev.c:1712
 0xb4374a do_compile
 ../../../src/gcc/gcc/toplev.c:1900
 Please submit a full bug report,
 with preprocessed source if appropriate.
 Please include the complete backtrace with any bug report.
 See http://gcc.gnu.org/bugs.html for instructions.


 I think we can close PR55113 and open a new bug for i386 and fshort-double
 usage if there isn't one open already?

 OK to commit and close bug?

It works fine on i386 for me but fails on x86_64.  Please add a

/* { dg-skip-if PR-you-open { { x86_64-*-* i?86-*-* }  lp64 } {
* } {  } } */

to the testcase to avoid the regression in the testsuite.

-fshort-double doesn't seem to work at all on 64bit x86.

Ok with that change.

Thanks,
Richard.

 --
 PMatos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-04 Thread Paulo J. Matos 

On 04/03/14 11:16, Richard Biener wrote:

It works fine on i386 for me but fails on x86_64.  Please add a

/* { dg-skip-if PR-you-open { { x86_64-*-* i?86-*-* }  lp64 } {
* } {  } } */

to the testcase to avoid the regression in the testsuite.



Apologies, you're right. I meant x86_64 fails here as well.
Will commit with suggested skip line.


--
PMatos

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-04 Thread Rainer Orth 
Paulo J. Matos pa...@matos-sorge.com writes:

 On 04/03/14 11:16, Richard Biener wrote:
 It works fine on i386 for me but fails on x86_64.  Please add a

 /* { dg-skip-if PR-you-open { { x86_64-*-* i?86-*-* }  lp64 } {
 * } {  } } */

 to the testcase to avoid the regression in the testsuite.


 Apologies, you're right. I meant x86_64 fails here as well.
 Will commit with suggested skip line.

Please omit the { * } {  } part.  That's the default and superfluous.

Thanks.
Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-03 Thread Richard Biener 
On Sat, Mar 1, 2014 at 11:23 PM, Paulo J. Matos pa...@matos-sorge.com wrote:

 This patch fixes lto/55113 for powerpc.
 Combining -fshort-double with -flto is now working fine.

 I attach patch and testcase (unsure if testcase is in the right place).
 Tested with target powerpc-abispe.


 2014-03-01  Paulo Matos  pa...@matos-sorge.com

 * c-family/c.opt: Add LTO FE support for fshort-double option.
 * tree-streamer.c (record_common_node): Assert we don't record
 nodes with type double.
 (preload_common_node): Skip type double, complex double and
 double pointer since it is now frontend dependent due to
 fshort-double option.

 2014-03-01  Paulo Matos  pa...@matos-sorge.com

 * gcc.target/powerpc/pr55113.c: New testcase.


 OK to commit?

Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt  (revision 208249)
+++ gcc/c-family/c.opt  (working copy)
@@ -1141,7 +1141,7 @@ C++ ObjC++ Optimization Var(flag_rtti) I
 Generate run time type descriptor information

 fshort-double
-C ObjC C++ ObjC++ Optimization Var(flag_short_double)
+C ObjC C++ ObjC++ LTO Optimization Var(flag_short_double)
 Use the same size for double as for float

This hunk isn't needed.

Index: gcc/testsuite/gcc.target/powerpc/pr55113.c
===
--- gcc/testsuite/gcc.target/powerpc/pr55113.c  (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr55113.c  (working copy)
@@ -0,0 +1,11 @@
+#include stdio.h
+
+int main()
+{
+   static float f;
+   float a = 1.0;
+   float b = 2.0;
+   f = a + b * 1e-12;
+   printf(%f\n, f);
+   return 0;
+}

that doesn't seem to be run with -flto nor -fshort-double.  The proper
place for a testcase is gcc.dg/lto/ with sth like

{ dg-lto-do link }
{ dg-lto-options { { -O2 -fshort-double -flto } } }

and naming the testcase pr55113_0.c.  Your testcase doens't use
doubles at all ... (well, ok, a + b * 1e-12 uses them implicitely, but
for that we have fsingle-precision-constant)

Richard.

 --
 PMatos

[PATCH] [lto/55113] Fix use of -fshort-double with -flto for powerpc

2014-03-01 Thread Paulo J. Matos 


This patch fixes lto/55113 for powerpc.
Combining -fshort-double with -flto is now working fine.

I attach patch and testcase (unsure if testcase is in the right place).
Tested with target powerpc-abispe.


2014-03-01  Paulo Matos  pa...@matos-sorge.com

* c-family/c.opt: Add LTO FE support for fshort-double option.
* tree-streamer.c (record_common_node): Assert we don't record
nodes with type double.
(preload_common_node): Skip type double, complex double and
double pointer since it is now frontend dependent due to
fshort-double option.

2014-03-01  Paulo Matos  pa...@matos-sorge.com

* gcc.target/powerpc/pr55113.c: New testcase.


OK to commit?

--
PMatos
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt	(revision 208249)
+++ gcc/c-family/c.opt	(working copy)
@@ -1141,7 +1141,7 @@ C++ ObjC++ Optimization Var(flag_rtti) I
 Generate run time type descriptor information
 
 fshort-double
-C ObjC C++ ObjC++ Optimization Var(flag_short_double)
+C ObjC C++ ObjC++ LTO Optimization Var(flag_short_double)
 Use the same size for double as for float
 
 fshort-enums
Index: gcc/tree-streamer.c
===
--- gcc/tree-streamer.c	(revision 208249)
+++ gcc/tree-streamer.c	(working copy)
@@ -264,7 +264,8 @@ record_common_node (struct streamer_tree
 
   gcc_checking_assert (node != boolean_type_node
 		node != boolean_true_node
-		node != boolean_false_node);
+		node != boolean_false_node
+		node != double_type_node);
 
   /* We have to make sure to fill exactly the same number of
  elements for all frontends.  That can include NULL trees.
@@ -315,10 +316,14 @@ preload_common_nodes (struct streamer_tr
 record_common_node (cache, sizetype_tab[i]);
 
   for (i = 0; i  TI_MAX; i++)
-/* Skip boolean type and constants, they are frontend dependent.  */
+/* Skip boolean type and constants. They are frontend dependent.
+   Skip double type, frontend dependent due to -fshort-double.  */
 if (i != TI_BOOLEAN_TYPE
 	 i != TI_BOOLEAN_FALSE
-	 i != TI_BOOLEAN_TRUE)
+	 i != TI_BOOLEAN_TRUE
+	 i != TI_DOUBLE_TYPE
+	 i != TI_COMPLEX_DOUBLE_TYPE
+	 i != TI_DOUBLE_PTR_TYPE)
   record_common_node (cache, global_trees[i]);
 }
 
Index: gcc/testsuite/gcc.target/powerpc/pr55113.c
===
--- gcc/testsuite/gcc.target/powerpc/pr55113.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr55113.c	(working copy)
@@ -0,0 +1,11 @@
+#include stdio.h
+
+int main()
+{
+	static float f;
+	float a = 1.0;
+	float b = 2.0;
+	f = a + b * 1e-12;
+	printf(%f\n, f);
+	return 0;
+}