| Message ID | 23849d2b-9339-95d8-8589-f9fdea4b8e9e@netcologne.de |
|---|---|
| State | New |
| Headers | show |
| Series | [fortran] PR 84487 | expand |
On Wed, Sep 25, 2019 at 10:24:51PM +0200, Thomas Koenig wrote: > > this patch makes sure that the __def_init variables, which have been > generated for normal allocatable arrays for quite some time, do not fill > up huge amounts of space in the object files with zeros. This is done by > not marking them read-only, which means that they are put into the BSS. > > Setting DECL_ARTIFICIAL on the __def_init variable makes sure it > is handled as predetermined shared in gfc_omp_predetermined_sharing . > > This is not an optimum solution. As the xfail shows, we are now missing > out on an optimization (as seen by the xfail that is now needed), and > having large all-zero variables seems wrong. However, this patch solves > the most urgent problem in this respect. > > This is an 8/9/10 regression, so I would like to commit this to > all of these branches (waiting before gcc 9 reopens, of course). > > I wold then close the PR and open an enchancement PR for the xfail > and the design improvement. > > Test case... I'm not sure what to test for. > > Regression-tested. OK for all affected branches? > Your approach seems reasonable to me. You may want to ping Jakub or Tobias as this affects openmp.
Hi, Jakub, do you have any comments? After Seve's OK I plan to commit in a couple of days unless I read anything to the contrary. Regards Thomas >> this patch makes sure that the __def_init variables, which have been >> generated for normal allocatable arrays for quite some time, do not fill >> up huge amounts of space in the object files with zeros. This is done by >> not marking them read-only, which means that they are put into the BSS. >> >> Setting DECL_ARTIFICIAL on the __def_init variable makes sure it >> is handled as predetermined shared in gfc_omp_predetermined_sharing . >> >> This is not an optimum solution. As the xfail shows, we are now missing >> out on an optimization (as seen by the xfail that is now needed), and >> having large all-zero variables seems wrong. However, this patch solves >> the most urgent problem in this respect. >> >> This is an 8/9/10 regression, so I would like to commit this to >> all of these branches (waiting before gcc 9 reopens, of course). >> >> I wold then close the PR and open an enchancement PR for the xfail >> and the design improvement. >> >> Test case... I'm not sure what to test for. >> >> Regression-tested. OK for all affected branches? >> > > Your approach seems reasonable to me. You may want to > ping Jakub or Tobias as this affects openmp. >
Index: fortran/trans-decl.c =================================================================== --- fortran/trans-decl.c (Revision 275719) +++ fortran/trans-decl.c (Arbeitskopie) @@ -1911,9 +1911,13 @@ gfc_get_symbol_decl (gfc_symbol * sym) if (sym->attr.associate_var) GFC_DECL_ASSOCIATE_VAR_P (decl) = 1; + /* We no longer mark __def_init as read-only so it does not take up + space in the read-only section and dan go into the BSS instead, + see PR 84487. Marking this as artificial means that OpenMP will + treat this as predetermined shared. */ if (sym->attr.vtab || (sym->name[0] == '_' && gfc_str_startswith (sym->name, "__def_init"))) - TREE_READONLY (decl) = 1; + DECL_ARTIFICIAL (decl) = 1; return decl; } Index: testsuite/gfortran.dg/typebound_call_22.f03 =================================================================== --- testsuite/gfortran.dg/typebound_call_22.f03 (Revision 275713) +++ testsuite/gfortran.dg/typebound_call_22.f03 (Arbeitskopie) @@ -26,4 +26,4 @@ program test call x%bar () end program -! { dg-final { scan-tree-dump-times "base \\(\\);" 1 "optimized" } } +! { dg-final { scan-tree-dump-times "base \\(\\);" 1 "optimized" { xfail *-*-* } } }