public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: New modref/ipa_modref optimization passes
       [not found]   ` <20200920061551.GA29500@kam.mff.cuni.cz>
@ 2020-09-21  7:13     ` Richard Biener
  2020-09-21  7:47       ` Jan Hubicka
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-21  7:13 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: David Čepelík, gcc-patches, fortran

On Sun, 20 Sep 2020, Jan Hubicka wrote:

> Hi,
> this is patch I am using to fix the assumed_alias_type.f90 failure by
> simply arranging alias set 0 for the problematic array descriptor.

There's no such named testcase on trunk.  Can you be more specific
as to the problem at hand?  It looks like gfortran.dg/assumed_type_9.f90
execute FAILs at the moment.

In particular how's this not an issue w/o IPA modref?

For TYPE(*) I think the object itself cannot be accessed but for
arrays the meta-info in the array descriptor can.  Now my question
would be why the Fortran FE at the call site does not build an
appropriately typed array descriptor?

CCing the fortran list.

> I am not sure this is the best option, but I suppose it is better than
> setting all array descritors to have same canonical type (as done by
> LTO)?
> 
> Honza
> 
> 	* trans-types.c (gfc_get_array_type_bounds): Set alias set to 0 for
> 	arrays of unknown element type.
> diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
> index 26fdb2803a7..bef3d270c06 100644
> --- a/gcc/fortran/trans-types.c
> +++ b/gcc/fortran/trans-types.c
> @@ -1903,6 +1903,12 @@ gfc_get_array_type_bounds (tree etype, int dimen, int codimen, tree * lbound,
>    base_type = gfc_get_array_descriptor_base (dimen, codimen, false);
>    TYPE_CANONICAL (fat_type) = base_type;
>    TYPE_STUB_DECL (fat_type) = TYPE_STUB_DECL (base_type);
> +  /* Arrays of unknown type must alias with all array descriptors.  */
> +  if (etype == ptr_type_node)
> +    {
> +      TYPE_ALIAS_SET (base_type) = 0;
> +      TYPE_ALIAS_SET (fat_type) = 0;
> +    }
>  
>    tmp = TYPE_NAME (etype);
>    if (tmp && TREE_CODE (tmp) == TYPE_DECL)
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-21  7:13     ` New modref/ipa_modref optimization passes Richard Biener
@ 2020-09-21  7:47       ` Jan Hubicka
  2020-09-21  7:55         ` Richard Biener
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Hubicka @ 2020-09-21  7:47 UTC (permalink / raw)
  To: Richard Biener; +Cc: David Čepelík, gcc-patches, fortran

> On Sun, 20 Sep 2020, Jan Hubicka wrote:
> 
> > Hi,
> > this is patch I am using to fix the assumed_alias_type.f90 failure by
> > simply arranging alias set 0 for the problematic array descriptor.
> 
> There's no such named testcase on trunk.  Can you be more specific
> as to the problem at hand?  It looks like gfortran.dg/assumed_type_9.f90
> execute FAILs at the moment.
> 
> In particular how's this not an issue w/o IPA modref?

> 
> For TYPE(*) I think the object itself cannot be accessed but for
> arrays the meta-info in the array descriptor can.  Now my question
> would be why the Fortran FE at the call site does not build an
> appropriately typed array descriptor?
> 
> CCing the fortran list.

The problem is:

alsize (struct array15_unknown & restrict a)
{
...
  _2 = *a_13(D).dtype.rank;
  _3 = (integer(kind=8)) _2;
...
}
}
and in main:

  struct array02_integer(kind=4) am;
  <bb 6> :
  MEM <c_char[8]> [(struct dtype_type *)&am + 24B] = {};
  am.dtype.elem_len = 4;
  am.dtype.rank = 2;
  am.dtype.type = 1;
...
  _52 = alsize (&am);

Here array15_unknown and array02_integer are different structures with
different canonical types and thus we end up disambiguating the accesses
via base alias sets.

My understanding is that this _unknown array descriptor is supposed to
be universal and work with all kinds of arrays.

Wihtout modref this works because alsize is not inlined (we think code
size would grow). Forcing inliner to inline stil leads to working code
because we first constant propagate the pointer and then we see accesses
from same base DECL thus bypass the TBAA checks.  Disabling the
constant propagation leads to wrong code as wel.

Honza

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-21  7:47       ` Jan Hubicka
@ 2020-09-21  7:55         ` Richard Biener
  2020-09-21  8:04           ` Jan Hubicka
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-21  7:55 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: David Čepelík, gcc-patches, fortran

On Mon, 21 Sep 2020, Jan Hubicka wrote:

> > On Sun, 20 Sep 2020, Jan Hubicka wrote:
> > 
> > > Hi,
> > > this is patch I am using to fix the assumed_alias_type.f90 failure by
> > > simply arranging alias set 0 for the problematic array descriptor.
> > 
> > There's no such named testcase on trunk.  Can you be more specific
> > as to the problem at hand?  It looks like gfortran.dg/assumed_type_9.f90
> > execute FAILs at the moment.
> > 
> > In particular how's this not an issue w/o IPA modref?
> 
> > 
> > For TYPE(*) I think the object itself cannot be accessed but for
> > arrays the meta-info in the array descriptor can.  Now my question
> > would be why the Fortran FE at the call site does not build an
> > appropriately typed array descriptor?
> > 
> > CCing the fortran list.
> 
> The problem is:
> 
> alsize (struct array15_unknown & restrict a)
> {
> ...
>   _2 = *a_13(D).dtype.rank;
>   _3 = (integer(kind=8)) _2;
> ...
> }
> }
> and in main:
> 
>   struct array02_integer(kind=4) am;
>   <bb 6> :
>   MEM <c_char[8]> [(struct dtype_type *)&am + 24B] = {};
>   am.dtype.elem_len = 4;
>   am.dtype.rank = 2;
>   am.dtype.type = 1;
> ...
>   _52 = alsize (&am);
> 
> Here array15_unknown and array02_integer are different structures with
> different canonical types and thus we end up disambiguating the accesses
> via base alias sets.
> 
> My understanding is that this _unknown array descriptor is supposed to
> be universal and work with all kinds of arrays.

But the FE builds a new descriptor for each individual call and thus
should build a universal descriptor for a call to an universal
descriptor argument.

Richard.

> Wihtout modref this works because alsize is not inlined (we think code
> size would grow). Forcing inliner to inline stil leads to working code
> because we first constant propagate the pointer and then we see accesses
> from same base DECL thus bypass the TBAA checks.  Disabling the
> constant propagation leads to wrong code as wel.
> 
> Honza
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-21  7:55         ` Richard Biener
@ 2020-09-21  8:04           ` Jan Hubicka
  2020-09-21  8:10             ` Richard Biener
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Hubicka @ 2020-09-21  8:04 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, fortran

> > 
> > The problem is:
> > 
> > alsize (struct array15_unknown & restrict a)
> > {
> > ...
> >   _2 = *a_13(D).dtype.rank;
> >   _3 = (integer(kind=8)) _2;
> > ...
> > }
> > }
> > and in main:
> > 
> >   struct array02_integer(kind=4) am;
> >   <bb 6> :
> >   MEM <c_char[8]> [(struct dtype_type *)&am + 24B] = {};
> >   am.dtype.elem_len = 4;
> >   am.dtype.rank = 2;
> >   am.dtype.type = 1;
> > ...
> >   _52 = alsize (&am);
> > 
> > Here array15_unknown and array02_integer are different structures with
> > different canonical types and thus we end up disambiguating the accesses
> > via base alias sets.
> > 
> > My understanding is that this _unknown array descriptor is supposed to
> > be universal and work with all kinds of arrays.
> 
> But the FE builds a new descriptor for each individual call and thus
> should build a universal descriptor for a call to an universal
> descriptor argument.

I see, so you would expect call to alsize to initialize things in
array15_unkonwn type?  That would work too.

Honza

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-21  8:04           ` Jan Hubicka
@ 2020-09-21  8:10             ` Richard Biener
  2020-09-22 10:15               ` Tobias Burnus
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-21  8:10 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, fortran

On Mon, 21 Sep 2020, Jan Hubicka wrote:

> > > 
> > > The problem is:
> > > 
> > > alsize (struct array15_unknown & restrict a)
> > > {
> > > ...
> > >   _2 = *a_13(D).dtype.rank;
> > >   _3 = (integer(kind=8)) _2;
> > > ...
> > > }
> > > }
> > > and in main:
> > > 
> > >   struct array02_integer(kind=4) am;
> > >   <bb 6> :
> > >   MEM <c_char[8]> [(struct dtype_type *)&am + 24B] = {};
> > >   am.dtype.elem_len = 4;
> > >   am.dtype.rank = 2;
> > >   am.dtype.type = 1;
> > > ...
> > >   _52 = alsize (&am);
> > > 
> > > Here array15_unknown and array02_integer are different structures with
> > > different canonical types and thus we end up disambiguating the accesses
> > > via base alias sets.
> > > 
> > > My understanding is that this _unknown array descriptor is supposed to
> > > be universal and work with all kinds of arrays.
> > 
> > But the FE builds a new descriptor for each individual call and thus
> > should build a universal descriptor for a call to an universal
> > descriptor argument.
> 
> I see, so you would expect call to alsize to initialize things in
> array15_unkonwn type?  That would work too.

Yes, that's my expectation.  But let's see what fortran folks say.

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-21  8:10             ` Richard Biener
@ 2020-09-22 10:15               ` Tobias Burnus
  2020-09-27 21:37                 ` Jan Hubicka
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Burnus @ 2020-09-22 10:15 UTC (permalink / raw)
  To: Richard Biener, Jan Hubicka; +Cc: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

On 9/21/20 10:10 AM, Richard Biener wrote:

>> I see, so you would expect call to alsize to initialize things in
>> array15_unkonwn type?  That would work too.
> Yes, that's my expectation.  But let's see what fortran folks say.

RFC patch attached; I think the following should work, but I am not
sure whether I missed something.

I wonder what to do about
   '!GCC$ NO_ARG_CHECK :: x
but that seems to work fine (creates void* type) and as it only
permits assumed size or scalar variables, the descriptor issue
does not occur.

Thoughts?

Tobias


[-- Attachment #2: assumed-type.diff --]
[-- Type: text/x-patch, Size: 3454 bytes --]

gcc/fortran/ChangeLog:

	* trans-array.c (gfc_conv_expr_descriptor):
	(gfc_conv_array_parameter):
	* trans-array.h (gfc_conv_expr_descriptor):

 gcc/fortran/trans-array.c | 15 +++++++++------
 gcc/fortran/trans-array.h |  3 ++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 6566c47d4ae..a5d1b477a0a 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -7216,7 +7216,7 @@ walk_coarray (gfc_expr *e)
    function call.  */
 
 void
-gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
+gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr, bool want_assumed_type)
 {
   gfc_ss *ss;
   gfc_ss_type ss_type;
@@ -7611,7 +7611,9 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
       else
 	{
 	  /* Otherwise make a new one.  */
-	  if (expr->ts.type == BT_CHARACTER && expr->ts.deferred)
+	  if (want_assumed_type)
+	    parmtype = ptr_type_node;
+	  else if (expr->ts.type == BT_CHARACTER && expr->ts.deferred)
 	    parmtype = gfc_typenode_for_spec (&expr->ts);
 	  else
 	    parmtype = gfc_get_element_type (TREE_TYPE (desc));
@@ -7950,7 +7952,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
         {
 	  if (sym->attr.dummy || sym->attr.result)
 	    {
-	      gfc_conv_expr_descriptor (se, expr);
+	      gfc_conv_expr_descriptor (se, expr,
+					fsym && fsym->ts.type == BT_ASSUMED);
 	      tmp = se->expr;
 	    }
 	  if (size)
@@ -8014,7 +8017,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
 
   if (no_pack || array_constructor || good_allocatable || ultimate_alloc_comp)
     {
-      gfc_conv_expr_descriptor (se, expr);
+      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
       /* Deallocate the allocatable components of structures that are
 	 not variable.  */
       if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
@@ -8037,7 +8040,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
   if (this_array_result)
     {
       /* Result of the enclosing function.  */
-      gfc_conv_expr_descriptor (se, expr);
+      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
       if (size)
 	array_parameter_size (se->expr, expr, size);
       se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
@@ -8053,7 +8056,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
     {
       /* Every other type of array.  */
       se->want_pointer = 1;
-      gfc_conv_expr_descriptor (se, expr);
+      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
 
       if (size)
 	array_parameter_size (build_fold_indirect_ref_loc (input_location,
diff --git a/gcc/fortran/trans-array.h b/gcc/fortran/trans-array.h
index e561605aaed..be3b1b79860 100644
--- a/gcc/fortran/trans-array.h
+++ b/gcc/fortran/trans-array.h
@@ -143,7 +143,8 @@ void gfc_get_dataptr_offset (stmtblock_t*, tree, tree, tree, bool, gfc_expr*);
 /* Obtain the span of an array.  */
 tree gfc_get_array_span (tree, gfc_expr *);
 /* Evaluate an array expression.  */
-void gfc_conv_expr_descriptor (gfc_se *, gfc_expr *);
+void gfc_conv_expr_descriptor (gfc_se *, gfc_expr *,
+			       bool want_assumed_type = false);
 /* Convert an array for passing as an actual function parameter.  */
 void gfc_conv_array_parameter (gfc_se *, gfc_expr *, bool,
 			       const gfc_symbol *, const char *, tree *);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-22 10:15               ` Tobias Burnus
@ 2020-09-27 21:37                 ` Jan Hubicka
  2020-10-16  7:56                   ` Jan Hubicka
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Hubicka @ 2020-09-27 21:37 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Richard Biener, gcc-patches, fortran

> On 9/21/20 10:10 AM, Richard Biener wrote:
> 
> > > I see, so you would expect call to alsize to initialize things in
> > > array15_unkonwn type?  That would work too.
> > Yes, that's my expectation.  But let's see what fortran folks say.
> 
> RFC patch attached; I think the following should work, but I am not
> sure whether I missed something.
> 
> I wonder what to do about
>   '!GCC$ NO_ARG_CHECK :: x
> but that seems to work fine (creates void* type) and as it only
> permits assumed size or scalar variables, the descriptor issue
> does not occur.
> 
> Thoughts?

Hi,
with somewhat improved ipa-modref and your patch i get following
failures:
FAIL: gfortran.dg/assumed_type_2.f90   -O   scan-tree-dump-times original "sub_array_assumed \\\\(\\\\(struct t1.0:. .\\\\) parm" 1
FAIL: gfortran.dg/assumed_type_9.f90   -O2  execution test
FAIL: gfortran.dg/assumed_type_9.f90   -Os  execution test
FAIL: gfortran.dg/class_allocate_20.f90   -O2  execution test
FAIL: gfortran.dg/class_allocate_20.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/class_allocate_20.f90   -O3 -g  execution test
FAIL: gfortran.dg/class_allocate_20.f90   -Os  execution test
FAIL: gfortran.dg/finalize_25.f90   -O2  execution test
FAIL: gfortran.dg/finalize_25.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/finalize_25.f90   -O3 -g  execution test
FAIL: gfortran.dg/finalize_25.f90   -Os  execution test
FAIL: gfortran.dg/no_arg_check_2.f90   -O   scan-tree-dump-times original "sub_array_assumed \\\\(\\\\(struct t1.0:. .\\\\) parm" 1
WARNING: gfortran.dg/pdt_14.f03   -O2  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O2  execution test
WARNING: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
WARNING: gfortran.dg/pdt_14.f03   -O3 -g  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O3 -g  execution test
WARNING: gfortran.dg/pdt_14.f03   -Os  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -Os  execution test
FAIL: gfortran.dg/sizeof_4.f90   -O0  execution test
FAIL: gfortran.dg/sizeof_4.f90   -O1  execution test
FAIL: gfortran.dg/sizeof_4.f90   -O2  execution test
FAIL: gfortran.dg/sizeof_4.f90   -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/sizeof_4.f90   -O3 -g  execution test
FAIL: gfortran.dg/sizeof_4.f90   -Os  execution test

With asumed_type_9.f90 we get:
__final_test_T4 (struct array15_t4 & restrict array, integer(kind=8) byte_stride, logical(kind=1) fini_coarray)

called as:

struct array00_t1 decl_uidesc.19
....
__final_test_T4 (&desc.19, 24, 0);

and we optimize out initializer of desc.19 since it is TBAA
incompatible (so same problem as with assumed type but this time the
consumer descriptor is not universal; just different).


With finalize_25 I see:

__final_gn_Sl (struct array15_sl & restrict array, integer(kind=8) byte_stride, logical(kind=1) fini_coarray)

called as:

struct array00_sl desc.20
...
__final_gn_Sl (&desc.20, 64, 0);


With pdf14_f03 I get disambiguation 
ipa-modref: in main/8, call to push_8/6 does not clobber __vtab_link_module_Pdtlink_8._deallocate 14->13
so this seems different and I am not quite sure what is wrong here.

FAIL: gfortran.dg/sizeof_4.f90   -O1  execution test

actually goes away with reverting your patch.

Honza
> 
> Tobias
> 

> gcc/fortran/ChangeLog:
> 
> 	* trans-array.c (gfc_conv_expr_descriptor):
> 	(gfc_conv_array_parameter):
> 	* trans-array.h (gfc_conv_expr_descriptor):
> 
>  gcc/fortran/trans-array.c | 15 +++++++++------
>  gcc/fortran/trans-array.h |  3 ++-
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
> index 6566c47d4ae..a5d1b477a0a 100644
> --- a/gcc/fortran/trans-array.c
> +++ b/gcc/fortran/trans-array.c
> @@ -7216,7 +7216,7 @@ walk_coarray (gfc_expr *e)
>     function call.  */
>  
>  void
> -gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
> +gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr, bool want_assumed_type)
>  {
>    gfc_ss *ss;
>    gfc_ss_type ss_type;
> @@ -7611,7 +7611,9 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
>        else
>  	{
>  	  /* Otherwise make a new one.  */
> -	  if (expr->ts.type == BT_CHARACTER && expr->ts.deferred)
> +	  if (want_assumed_type)
> +	    parmtype = ptr_type_node;
> +	  else if (expr->ts.type == BT_CHARACTER && expr->ts.deferred)
>  	    parmtype = gfc_typenode_for_spec (&expr->ts);
>  	  else
>  	    parmtype = gfc_get_element_type (TREE_TYPE (desc));
> @@ -7950,7 +7952,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
>          {
>  	  if (sym->attr.dummy || sym->attr.result)
>  	    {
> -	      gfc_conv_expr_descriptor (se, expr);
> +	      gfc_conv_expr_descriptor (se, expr,
> +					fsym && fsym->ts.type == BT_ASSUMED);
>  	      tmp = se->expr;
>  	    }
>  	  if (size)
> @@ -8014,7 +8017,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
>  
>    if (no_pack || array_constructor || good_allocatable || ultimate_alloc_comp)
>      {
> -      gfc_conv_expr_descriptor (se, expr);
> +      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
>        /* Deallocate the allocatable components of structures that are
>  	 not variable.  */
>        if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
> @@ -8037,7 +8040,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
>    if (this_array_result)
>      {
>        /* Result of the enclosing function.  */
> -      gfc_conv_expr_descriptor (se, expr);
> +      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
>        if (size)
>  	array_parameter_size (se->expr, expr, size);
>        se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
> @@ -8053,7 +8056,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
>      {
>        /* Every other type of array.  */
>        se->want_pointer = 1;
> -      gfc_conv_expr_descriptor (se, expr);
> +      gfc_conv_expr_descriptor (se, expr, fsym && fsym->ts.type == BT_ASSUMED);
>  
>        if (size)
>  	array_parameter_size (build_fold_indirect_ref_loc (input_location,
> diff --git a/gcc/fortran/trans-array.h b/gcc/fortran/trans-array.h
> index e561605aaed..be3b1b79860 100644
> --- a/gcc/fortran/trans-array.h
> +++ b/gcc/fortran/trans-array.h
> @@ -143,7 +143,8 @@ void gfc_get_dataptr_offset (stmtblock_t*, tree, tree, tree, bool, gfc_expr*);
>  /* Obtain the span of an array.  */
>  tree gfc_get_array_span (tree, gfc_expr *);
>  /* Evaluate an array expression.  */
> -void gfc_conv_expr_descriptor (gfc_se *, gfc_expr *);
> +void gfc_conv_expr_descriptor (gfc_se *, gfc_expr *,
> +			       bool want_assumed_type = false);
>  /* Convert an array for passing as an actual function parameter.  */
>  void gfc_conv_array_parameter (gfc_se *, gfc_expr *, bool,
>  			       const gfc_symbol *, const char *, tree *);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-09-27 21:37                 ` Jan Hubicka
@ 2020-10-16  7:56                   ` Jan Hubicka
  2020-10-16  8:05                     ` Richard Biener
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Hubicka @ 2020-10-16  7:56 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Richard Biener, gcc-patches, fortran

Hi,
I am slowly getting finished with the fn spec changes on trunk and then
would like to proceed with modref.  Sadly  still get the assumed_type
failuere and in addition to that:
FAIL: gfortran.dg/finalize_25.f90   -O2  execution test
FAIL: gfortran.dg/finalize_25.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/finalize_25.f90   -O3 -g  execution test
FAIL: gfortran.dg/finalize_25.f90   -Os  execution test
WARNING: gfortran.dg/pdt_14.f03   -O2  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O2  execution test
WARNING: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
WARNING: gfortran.dg/pdt_14.f03   -O3 -g  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -O3 -g  execution test
WARNING: gfortran.dg/pdt_14.f03   -Os  execution test program timed out.
FAIL: gfortran.dg/pdt_14.f03   -Os  execution test

I wonder if there is any chance to get Fortran FE fixed here?

Honza

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-10-16  7:56                   ` Jan Hubicka
@ 2020-10-16  8:05                     ` Richard Biener
  2020-10-16  9:20                       ` Richard Biener
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-10-16  8:05 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Tobias Burnus, gcc-patches, fortran

On Fri, 16 Oct 2020, Jan Hubicka wrote:

> Hi,
> I am slowly getting finished with the fn spec changes on trunk and then
> would like to proceed with modref.  Sadly  still get the assumed_type
> failuere and in addition to that:
> FAIL: gfortran.dg/finalize_25.f90   -O2  execution test
> FAIL: gfortran.dg/finalize_25.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> FAIL: gfortran.dg/finalize_25.f90   -O3 -g  execution test
> FAIL: gfortran.dg/finalize_25.f90   -Os  execution test
> WARNING: gfortran.dg/pdt_14.f03   -O2  execution test program timed out.
> FAIL: gfortran.dg/pdt_14.f03   -O2  execution test
> WARNING: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> program timed out.
> FAIL: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> WARNING: gfortran.dg/pdt_14.f03   -O3 -g  execution test program timed out.
> FAIL: gfortran.dg/pdt_14.f03   -O3 -g  execution test
> WARNING: gfortran.dg/pdt_14.f03   -Os  execution test program timed out.
> FAIL: gfortran.dg/pdt_14.f03   -Os  execution test
> 
> I wonder if there is any chance to get Fortran FE fixed here?

OK, I'll try doing a little surgery in the FE today, coming up with
a little refactoring and a fix along your original one that allows
for a better one by FE folks.

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-10-16  8:05                     ` Richard Biener
@ 2020-10-16  9:20                       ` Richard Biener
  2020-10-16 10:42                         ` Richard Biener
  2020-10-23  9:54                         ` Bernhard Reutner-Fischer
  0 siblings, 2 replies; 13+ messages in thread
From: Richard Biener @ 2020-10-16  9:20 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Tobias Burnus, gcc-patches, fortran

On Fri, 16 Oct 2020, Richard Biener wrote:

> On Fri, 16 Oct 2020, Jan Hubicka wrote:
> 
> > Hi,
> > I am slowly getting finished with the fn spec changes on trunk and then
> > would like to proceed with modref.  Sadly  still get the assumed_type
> > failuere and in addition to that:
> > FAIL: gfortran.dg/finalize_25.f90   -O2  execution test
> > FAIL: gfortran.dg/finalize_25.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > FAIL: gfortran.dg/finalize_25.f90   -O3 -g  execution test
> > FAIL: gfortran.dg/finalize_25.f90   -Os  execution test
> > WARNING: gfortran.dg/pdt_14.f03   -O2  execution test program timed out.
> > FAIL: gfortran.dg/pdt_14.f03   -O2  execution test
> > WARNING: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > program timed out.
> > FAIL: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > WARNING: gfortran.dg/pdt_14.f03   -O3 -g  execution test program timed out.
> > FAIL: gfortran.dg/pdt_14.f03   -O3 -g  execution test
> > WARNING: gfortran.dg/pdt_14.f03   -Os  execution test program timed out.
> > FAIL: gfortran.dg/pdt_14.f03   -Os  execution test
> > 
> > I wonder if there is any chance to get Fortran FE fixed here?
> 
> OK, I'll try doing a little surgery in the FE today, coming up with
> a little refactoring and a fix along your original one that allows
> for a better one by FE folks.

So I've sent a refactoring patch improving the tree building code.

But now trying to fix the actual issue with the idea to perform
accesses indirectly via a descriptor with dim[] type I see that
the coarray 'token' field is appended to descriptors conditional
on -fcoarray and that this field makes the dim[] array no longer
trailing - which means the offset of the 'token' field depends
on the rank of the array.

The dim[] field is even optional when dim + codimen == 0 and that
case indeed happens (ah, via get_scalar_to_descriptor_type).
So much for re-using this combo ;)

I suppose we can compensate for this by dynamically computing the
offset of the 'token' field but then it's not obvious to me
where the total 'rank' is stored inside the descriptor or how
the 'token' field is accessed for assumed-shape arrays - the
current method by simple field chaining definitely won't work.

Anyway, I'll try to deal with all this by just adjusting the TBAA
type but not the access type leaving that alone.

IMHO the cleanest way would be to swap the CAF token field and
the dim[] field (which is an ABI change for -fcoarray)

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-10-16  9:20                       ` Richard Biener
@ 2020-10-16 10:42                         ` Richard Biener
  2020-10-23  9:54                         ` Bernhard Reutner-Fischer
  1 sibling, 0 replies; 13+ messages in thread
From: Richard Biener @ 2020-10-16 10:42 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Tobias Burnus, gcc-patches, fortran

On Fri, 16 Oct 2020, Richard Biener wrote:

> On Fri, 16 Oct 2020, Richard Biener wrote:
> 
> > On Fri, 16 Oct 2020, Jan Hubicka wrote:
> > 
> > > Hi,
> > > I am slowly getting finished with the fn spec changes on trunk and then
> > > would like to proceed with modref.  Sadly  still get the assumed_type
> > > failuere and in addition to that:
> > > FAIL: gfortran.dg/finalize_25.f90   -O2  execution test
> > > FAIL: gfortran.dg/finalize_25.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > > FAIL: gfortran.dg/finalize_25.f90   -O3 -g  execution test
> > > FAIL: gfortran.dg/finalize_25.f90   -Os  execution test
> > > WARNING: gfortran.dg/pdt_14.f03   -O2  execution test program timed out.
> > > FAIL: gfortran.dg/pdt_14.f03   -O2  execution test
> > > WARNING: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > > program timed out.
> > > FAIL: gfortran.dg/pdt_14.f03   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > > WARNING: gfortran.dg/pdt_14.f03   -O3 -g  execution test program timed out.
> > > FAIL: gfortran.dg/pdt_14.f03   -O3 -g  execution test
> > > WARNING: gfortran.dg/pdt_14.f03   -Os  execution test program timed out.
> > > FAIL: gfortran.dg/pdt_14.f03   -Os  execution test
> > > 
> > > I wonder if there is any chance to get Fortran FE fixed here?
> > 
> > OK, I'll try doing a little surgery in the FE today, coming up with
> > a little refactoring and a fix along your original one that allows
> > for a better one by FE folks.
> 
> So I've sent a refactoring patch improving the tree building code.
> 
> But now trying to fix the actual issue with the idea to perform
> accesses indirectly via a descriptor with dim[] type I see that
> the coarray 'token' field is appended to descriptors conditional
> on -fcoarray and that this field makes the dim[] array no longer
> trailing - which means the offset of the 'token' field depends
> on the rank of the array.
> 
> The dim[] field is even optional when dim + codimen == 0 and that
> case indeed happens (ah, via get_scalar_to_descriptor_type).
> So much for re-using this combo ;)
> 
> I suppose we can compensate for this by dynamically computing the
> offset of the 'token' field but then it's not obvious to me
> where the total 'rank' is stored inside the descriptor or how
> the 'token' field is accessed for assumed-shape arrays - the
> current method by simple field chaining definitely won't work.
> 
> Anyway, I'll try to deal with all this by just adjusting the TBAA
> type but not the access type leaving that alone.
> 
> IMHO the cleanest way would be to swap the CAF token field and
> the dim[] field (which is an ABI change for -fcoarray)

OK, so I tried

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index f30a2f75701..29381f6756e 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -142,6 +142,17 @@ gfc_get_descriptor_field (tree desc, unsigned 
field_idx)
   tree field = gfc_advance_chain (TYPE_FIELDS (type), field_idx);
   gcc_assert (field != NULL_TREE);
 
+  /* We need to use a consistent descriptor type across all accesses
+     which should be possible by indirectly accessing the descriptor
+     via a type with a trailing flexible array dim[] member if there
+     were not the CAF token field after it.  So for now ensure correct
+     TBAA behavior by explicitely specifying a common TBAA type - any
+     descriptor-like type is OK here.  */
+  tree tbaa_type
+    = build_pointer_type (gfc_get_array_descriptor_base (4, 2, false));
+  desc = fold_build2_loc (input_location, MEM_REF, type,
+                         build_fold_addr_expr_loc (input_location, desc),
+                         build_int_cst (tbaa_type, 0));
   return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE 
(field),
                          desc, field, NULL_TREE);
 }

which first reveals two spots missed by the sent refactoring and
second exposes the fact that we use aggregate copies to copy
array descritors or aggregates with array descriptor typed fields.
There's currently no way to assign a custom TBAA type to aggregate
fields which means that the only choice is to fix things at the
above central place is using alias-set zero (and hoping for
the embedded/aggregate copied places to match up).  In the end
this means that the optimal approach is to adjust only those
accesses where we do not know the actual descriptor type but
I expect those to be spread out?  Eventually those cases
could be identified above?

Meanwhile the refactoring patch still applies of course,
amended by adjustments to gfc_conv_descriptor_data_addr
and gfc_conv_descriptor_data_set.

Unfortunately punning the descriptor like above causes
numerous testsuite FAILs due to orignal dump scannings no
longer matching :/  So I'm hoping for a hint as to how to
identify problematical descriptor types to reduce that
noise ...

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-10-16  9:20                       ` Richard Biener
  2020-10-16 10:42                         ` Richard Biener
@ 2020-10-23  9:54                         ` Bernhard Reutner-Fischer
  2020-10-23 10:05                           ` Andre Vehreschild
  1 sibling, 1 reply; 13+ messages in thread
From: Bernhard Reutner-Fischer @ 2020-10-23  9:54 UTC (permalink / raw)
  To: Richard Biener, Jan Hubicka; +Cc: Tobias Burnus, gcc-patches, fortran

On 16 October 2020 11:20:23 CEST, Richard Biener <rguenther@suse.de> wrote:

>IMHO the cleanest way would be to swap the CAF token field and
>the dim[] field (which is an ABI change for -fcoarray)

I think coarrays are new anyway so I suppose an ABI break is fine?


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: New modref/ipa_modref optimization passes
  2020-10-23  9:54                         ` Bernhard Reutner-Fischer
@ 2020-10-23 10:05                           ` Andre Vehreschild
  0 siblings, 0 replies; 13+ messages in thread
From: Andre Vehreschild @ 2020-10-23 10:05 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer via Fortran
  Cc: Bernhard Reutner-Fischer, Richard Biener, Jan Hubicka,
	Tobias Burnus, gcc-patches



On Fri, 23 Oct 2020 11:54:08 +0200
Bernhard Reutner-Fischer via Fortran <fortran@gcc.gnu.org> wrote:

> On 16 October 2020 11:20:23 CEST, Richard Biener <rguenther@suse.de> wrote:
>
> >IMHO the cleanest way would be to swap the CAF token field and
> >the dim[] field (which is an ABI change for -fcoarray)
>
> I think coarrays are new anyway so I suppose an ABI break is fine?

Coarrays are in the standard since Fortran 2008. So what I'd rather not call
them new being there for more than 10 years...

The descriptor is used in the opencoarrays library, too. And has to be kept in
sync. So when the ABI break is reasonable it's fine.

--
Andre Vehreschild * Email: vehre ad gmx dot de

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-23 10:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <157394261677.27454.2367573047582814412@a285.localdomain>
     [not found] ` <20200919223255.GA69342@kam.mff.cuni.cz>
     [not found]   ` <20200920061551.GA29500@kam.mff.cuni.cz>
2020-09-21  7:13     ` New modref/ipa_modref optimization passes Richard Biener
2020-09-21  7:47       ` Jan Hubicka
2020-09-21  7:55         ` Richard Biener
2020-09-21  8:04           ` Jan Hubicka
2020-09-21  8:10             ` Richard Biener
2020-09-22 10:15               ` Tobias Burnus
2020-09-27 21:37                 ` Jan Hubicka
2020-10-16  7:56                   ` Jan Hubicka
2020-10-16  8:05                     ` Richard Biener
2020-10-16  9:20                       ` Richard Biener
2020-10-16 10:42                         ` Richard Biener
2020-10-23  9:54                         ` Bernhard Reutner-Fischer
2020-10-23 10:05                           ` Andre Vehreschild

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).