Patchwork [Fortran] PR 44945 - Fix DECL of module variables (wrong-code "regression")

login
register
mail settings
Submitter Tobias Burnus
Date July 16, 2010, 2:04 p.m.
Message ID <4C4066F9.80902@net-b.de>
Download mbox | patch
Permalink /patch/59107/
State New
Headers show

Comments

Tobias Burnus - July 16, 2010, 2:04 p.m.
gfortran can produce the multiple declarations for module variables.
This causes problems when inlining is done as then the alias analysis
breaks.

For instance, the recent patch to the string intrinsics (marking some as
PURE) "caused" gfortran.dg/char_array_structure_constructor.f90 to break
with -m32 on x86_64-apple-darwin10.

The patch fixes this for -fwhole-file. Does anyone know why derived
types were excluded?

I have now added -fwhole-file to the test case to silence the error
(with this patch applied), but I think the real solution it to switch to
-fwhole-file by default.

Build and regtested on x86-64-linux. OK for the trunk? What about 4.5?

* * *

RFC: Is there any compelling reason not to switch to -fwhole-file by
default? I think we have slowly reached the state when there are more
bugs without that option than with that option.

Advantages
- Fixes some wrong-code issues
- Improves diagnostic
- Improves (non-LTO) optimizations
- More consistent codepath: The same for default, LTO and -fwhole-program

Disadvantage
- Some bugs, leading to wrong code

I think most wrong-code issues come apparent with -fwhole-program, which
allows for more optimizations - but that option implies -fwhole-file
(thus there is no change). Using -flto, one sees fewer problems as this
fixes some bad DECL.

Tobias
Paul Richard Thomas - July 16, 2010, 2:40 p.m.
Tobias,

OK for trunk and 4.5 after a delay of a week or two.

As I say elsewhere, I would be sure that the different treatment of
derived types came about form TYPE_CANONICAL not being set.

Thanks

Paul

On Fri, Jul 16, 2010 at 4:04 PM, Tobias Burnus <burnus@net-b.de> wrote:
> gfortran can produce the multiple declarations for module variables.
> This causes problems when inlining is done as then the alias analysis
> breaks.
>
> For instance, the recent patch to the string intrinsics (marking some as
> PURE) "caused" gfortran.dg/char_array_structure_constructor.f90 to break
> with -m32 on x86_64-apple-darwin10.
>
> The patch fixes this for -fwhole-file. Does anyone know why derived
> types were excluded?
>
> I have now added -fwhole-file to the test case to silence the error
> (with this patch applied), but I think the real solution it to switch to
> -fwhole-file by default.
>
> Build and regtested on x86-64-linux. OK for the trunk? What about 4.5?
>
> * * *
>
> RFC: Is there any compelling reason not to switch to -fwhole-file by
> default? I think we have slowly reached the state when there are more
> bugs without that option than with that option.
>
> Advantages
> - Fixes some wrong-code issues
> - Improves diagnostic
> - Improves (non-LTO) optimizations
> - More consistent codepath: The same for default, LTO and -fwhole-program
>
> Disadvantage
> - Some bugs, leading to wrong code
>
> I think most wrong-code issues come apparent with -fwhole-program, which
> allows for more optimizations - but that option implies -fwhole-file
> (thus there is no change). Using -flto, one sees fewer problems as this
> fixes some bad DECL.
>
> Tobias
>
Jakub Jelinek - July 16, 2010, 2:50 p.m.
On Fri, Jul 16, 2010 at 04:04:41PM +0200, Tobias Burnus wrote:
> 2010-07-16  Tobias Burnus  <burnus@net-b.de>
> 
> 	PR fortran/44945
> 	* gfortran.dg/char_array_structure_constructor.f90: Add
> 	-fwhole-file as dg-option as it otherwise fails on some
> 	systems.
> 
> --- gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90	(revision 162255)
> +++ gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90	(working copy)
> @@ -1,4 +1,9 @@
>  ! { dg-do run }
> +! { dg-options "-O3 -fwhole-file" }

Shouldn't this be just "-fwhole-file"?  Otherwise you test 4 times the same
command line, as -O3 comes after the -O? from cycling through the
optimization options.

	Jakub

Patch

2010-07-16  Tobias Burnus  <burnus@net-b.de>

	PR fortran/44945
	* trans-decl.c (gfc_get_symbol_decl): Use module decl with
	-fwhole-file also for derived types.

2010-07-16  Tobias Burnus  <burnus@net-b.de>

	PR fortran/44945
	* gfortran.dg/char_array_structure_constructor.f90: Add
	-fwhole-file as dg-option as it otherwise fails on some
	systems.

Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 162255)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -1149,11 +1149,9 @@  gfc_get_symbol_decl (gfc_symbol * sym)
     return sym->backend_decl;
 
   /* If use associated and whole file compilation, use the module
-     declaration.  This is only needed for intrinsic types because
-     they are substituted for one another during optimization.  */
+     declaration.  */
   if (gfc_option.flag_whole_file
 	&& sym->attr.flavor == FL_VARIABLE
-	&& sym->ts.type != BT_DERIVED
 	&& sym->attr.use_assoc
 	&& sym->module)
     {
Index: gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90
===================================================================
--- gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90	(revision 162255)
+++ gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90	(working copy)
@@ -1,4 +1,9 @@ 
 ! { dg-do run }
+! { dg-options "-O3 -fwhole-file" }
+!
+! PR fortran/19107
+! -fwhole-file flag added for PR fortran/44945
+!
 ! This test the fix of PR19107, where character array actual
 ! arguments in derived type constructors caused an ICE.
 ! It also checks that the scalar counterparts are OK.