Re: New modref/ipa_modref optimization passes

[Richard Biener <rguenther@suse.de>]

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.