[PATCH] Fix up Fortran debug info for arrays with descriptors (PR fortran/92775)

[PATCH] Fix up Fortran debug info for arrays with descriptors (PR fortran/92775)

[Jakub Jelinek]

Hi!

Before r251949 the debug info for Fortran array descriptors looked correct,
for arrays that had GFC_TYPE_ARRAY_SPAN NULL would use for DW_AT_byte_stride
DW_OP_push_object_address DW_OP_plus_uconst offsetof (, dim[0].__stride)
DW_OP_deref DW_OP_litN DW_OP_mul (where N is the element size in bytes constant),
and if GFC_TYPE_ARRAY_SPAN is non-NULL, some span.NN variable, then
DW_OP_push_object_address DW_OP_plus_uconst offsetof (, dim[0].__stride)
DW_OP_deref DW_OP_addr span.NN DW_OP_deref DW_OP_mul
Thus, e.g. first dimension's stride in bytes was computed as
descr->dim[0].__stride * element_size or
descr->dim[0].__stride * span.NN
With that change, the span field has been added to descriptors, but now we
always emit the
DW_OP_push_object_address DW_OP_plus_uconst offsetof (, dim[0].__stride)
DW_OP_deref DW_OP_litN DW_OP_mul (where N is the element size in bytes constant),
aka
descr->dim[0].__stride * element_size
which is incorrect for arrays where descr->span is different from
element_size.
I unfortunately don't see in TYPE_LANG_SPECIFIC anything left that would
record whether the compiler knows the type will certainly have descr->span
equal to element_size, or when it might not, so the following patch always
uses
DW_OP_push_object_address DW_OP_plus_uconst offsetof (, dim[0].__stride)
DW_OP_deref DW_OP_push_object_address DW_OP_plus_uconst offsetof (, span)
DW_OP_deref DW_OP_mul
aka
descr->dim[0].__stride * descr->span
which should be always? correct, but in the common case is 3 bytes longer
than really needed.

Unfortunately, I don't know Fortran enough to find out when exactly the
compiler knows that descr->span will be always equal to element_size, if
somebody could add a bool or unsigned : 1 field into struct lang_type,
corresponding macro and make sure it is set correctly, should be trivial to
tweak the patch so that if the compiler knows for sure it is a waste to
look up descr->span as it must be equal to elem_size to just
compute
  elem_size = fold_convert (gfc_array_index_type, TYPE_SIZE_UNIT (etype));
the way it used to be before.  Just it needs to be something stored on the
TYPE_LANG_SPECIFIC, as the type is all this hook is called with.

In the meantime, I think it is better to go for correct debug info over
saving the 3 bytes in .debug_info per dimension.

The patch also removes fields/macros that aren't ever used since r251949
and just waste compile time memory.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/92775
	* trans.h (struct lang_type, struct lang_decl): Remove span member.
	(GFC_DECL_SPAN, GFC_TYPE_ARRAY_SPAN): Remove macros.
	* trans-array.h (gfc_get_descriptor_offsets_for_info): Add another
	argument.
	* trans-array.c (gfc_get_descriptor_offsets_for_info): Add SPAN_OFF
	argument and initialize *SPAN_OFF to the offset of span field.
	* trans-types.c (gfc_get_array_descr_info): Adjust
	gfc_get_descriptor_offsets_for_info caller.  Compute elem_size
	as base->span instead of TYPE_SIZE_UNIT (etype) constant.

--- gcc/fortran/trans.h.jj	2019-11-11 21:04:05.210259346 +0100
+++ gcc/fortran/trans.h	2019-12-05 11:23:55.935237355 +0100
@@ -981,7 +981,6 @@ struct GTY(())	lang_type	 {
   tree offset;
   tree dtype;
   tree dataptr_type;
-  tree span;
   tree base_decl[2];
   tree nonrestricted_type;
   tree caf_token;
@@ -997,7 +996,6 @@ struct GTY(()) lang_decl {
      address of target label.  */
   tree stringlen;
   tree addr;
-  tree span;
   /* For assumed-shape coarrays.  */
   tree token, caf_offset;
   unsigned int scalar_allocatable : 1;
@@ -1008,7 +1006,6 @@ struct GTY(()) lang_decl {
 
 #define GFC_DECL_ASSIGN_ADDR(node) DECL_LANG_SPECIFIC(node)->addr
 #define GFC_DECL_STRING_LEN(node) DECL_LANG_SPECIFIC(node)->stringlen
-#define GFC_DECL_SPAN(node) DECL_LANG_SPECIFIC(node)->span
 #define GFC_DECL_TOKEN(node) DECL_LANG_SPECIFIC(node)->token
 #define GFC_DECL_CAF_OFFSET(node) DECL_LANG_SPECIFIC(node)->caf_offset
 #define GFC_DECL_SAVED_DESCRIPTOR(node) \
@@ -1059,7 +1056,6 @@ struct GTY(()) lang_decl {
 #define GFC_TYPE_ARRAY_DTYPE(node) (TYPE_LANG_SPECIFIC(node)->dtype)
 #define GFC_TYPE_ARRAY_DATAPTR_TYPE(node) \
   (TYPE_LANG_SPECIFIC(node)->dataptr_type)
-#define GFC_TYPE_ARRAY_SPAN(node) (TYPE_LANG_SPECIFIC(node)->span)
 #define GFC_TYPE_ARRAY_BASE_DECL(node, internal) \
   (TYPE_LANG_SPECIFIC(node)->base_decl[(internal)])
 
--- gcc/fortran/trans-array.h.jj	2019-09-26 22:02:40.236457478 +0200
+++ gcc/fortran/trans-array.h	2019-12-05 11:07:52.255125510 +0100
@@ -163,7 +163,7 @@ void gfc_trans_array_cobounds (tree, stm
 
 /* Build expressions for accessing components of an array descriptor.  */
 void gfc_get_descriptor_offsets_for_info (const_tree, tree *, tree *, tree *, tree *,
-					  tree *, tree *, tree *);
+					  tree *, tree *, tree *, tree *);
 
 tree gfc_conv_descriptor_data_get (tree);
 tree gfc_conv_descriptor_data_addr (tree);
--- gcc/fortran/trans-array.c.jj	2019-11-01 09:01:50.111998535 +0100
+++ gcc/fortran/trans-array.c	2019-12-05 11:09:38.545483494 +0100
@@ -540,9 +540,10 @@ gfc_conv_shift_descriptor_lbound (stmtbl
 
 void
 gfc_get_descriptor_offsets_for_info (const_tree desc_type, tree *data_off,
-				     tree *dtype_off, tree *dim_off,
-				     tree *dim_size, tree *stride_suboff,
-				     tree *lower_suboff, tree *upper_suboff)
+				     tree *dtype_off, tree *span_off,
+				     tree *dim_off, tree *dim_size,
+				     tree *stride_suboff, tree *lower_suboff,
+				     tree *upper_suboff)
 {
   tree field;
   tree type;
@@ -552,6 +553,8 @@ gfc_get_descriptor_offsets_for_info (con
   *data_off = byte_position (field);
   field = gfc_advance_chain (TYPE_FIELDS (type), DTYPE_FIELD);
   *dtype_off = byte_position (field);
+  field = gfc_advance_chain (TYPE_FIELDS (type), SPAN_FIELD);
+  *span_off = byte_position (field);
   field = gfc_advance_chain (TYPE_FIELDS (type), DIMENSION_FIELD);
   *dim_off = byte_position (field);
   type = TREE_TYPE (TREE_TYPE (field));
--- gcc/fortran/trans-types.c.jj	2019-08-27 12:27:05.678497543 +0200
+++ gcc/fortran/trans-types.c	2019-12-05 11:39:33.868747876 +0100
@@ -3266,7 +3266,7 @@ gfc_get_array_descr_info (const_tree typ
   int rank, dim;
   bool indirect = false;
   tree etype, ptype, t, base_decl;
-  tree data_off, dim_off, dtype_off, dim_size, elem_size;
+  tree data_off, span_off, dim_off, dtype_off, dim_size, elem_size;
   tree lower_suboff, upper_suboff, stride_suboff;
   tree dtype, field, rank_off;
 
@@ -3323,12 +3323,13 @@ gfc_get_array_descr_info (const_tree typ
   if (indirect)
     base_decl = build1 (INDIRECT_REF, ptype, base_decl);
 
-  elem_size = fold_convert (gfc_array_index_type, TYPE_SIZE_UNIT (etype));
-
-  gfc_get_descriptor_offsets_for_info (type, &data_off, &dtype_off, &dim_off,
-				       &dim_size, &stride_suboff,
+  gfc_get_descriptor_offsets_for_info (type, &data_off, &dtype_off, &span_off,
+				       &dim_off, &dim_size, &stride_suboff,
 				       &lower_suboff, &upper_suboff);
 
+  t = fold_build_pointer_plus (base_decl, span_off);
+  elem_size = build1 (INDIRECT_REF, gfc_array_index_type, t);
+
   t = base_decl;
   if (!integer_zerop (data_off))
     t = fold_build_pointer_plus (t, data_off);

	Jakub

Re: [PATCH] Fix up Fortran debug info for arrays with descriptors (PR fortran/92775)

[Tobias Burnus]

@Paul: I have an ISO-C question for you.


On 12/6/19 1:16 AM, Jakub Jelinek wrote:
> […]
> Unfortunately, I don't know Fortran enough to find out when exactly the
> compiler knows that descr->span will be always equal to element_size,

Well, while gfc_expr is still available, that's gfc_get_array_span; if 
it is neither a pointer (in the Fortran sense)/ISO-C descriptor nor a 
polymorphic, it has the element length [for strings: times the string 
length].

Instead of going for the general solution, I wonder whether one could 
use something like: If type is integer/real/complex and not a pointer, 
take the element size; namely, something like:

if (GFC_TYPE_ARRAY_AKIND (type) != GFC_ARRAY_POINTER
     && (INTEGRAL_TYPE_P (type) || SCALAR_FLOAT_TYPE_P (type)
	|| COMPLEX_FLOAT_TYPE_P (type)))

That wouldn't cover known-size/nonpolymorphic derived types nor compile-time
const strings (span = <unit size of char type> * <string length>) but at least
the ubiquitous numeric arrays.
It surely could be refined to cover the rest (fixed-length strings, nonpolymorphic
types).

I only don't quickly see which GFC_TYPE_ARRAY_AKIND the ISO-C descriptor
gets, i.e. whether condition above will work or whether something in addition
is needed.

Paul?

(For me the whole gfc <-> ISO C descriptor handling looks like a mess,
but I also have not yet tried to fully understood how it works in detail.)
  

> In the meantime, I think it is better to go for correct debug info over
> saving the 3 bytes in .debug_info per dimension.
>
> The patch also removes fields/macros that aren't ever used since r251949
> and just waste compile time memory.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK; thanks for the patch!

Tobias

> 2019-12-06  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR fortran/92775
> 	* trans.h (struct lang_type, struct lang_decl): Remove span member.
> 	(GFC_DECL_SPAN, GFC_TYPE_ARRAY_SPAN): Remove macros.
> 	* trans-array.h (gfc_get_descriptor_offsets_for_info): Add another
> 	argument.
> 	* trans-array.c (gfc_get_descriptor_offsets_for_info): Add SPAN_OFF
> 	argument and initialize *SPAN_OFF to the offset of span field.
> 	* trans-types.c (gfc_get_array_descr_info): Adjust
> 	gfc_get_descriptor_offsets_for_info caller.  Compute elem_size
> 	as base->span instead of TYPE_SIZE_UNIT (etype) constant.
>
> --- gcc/fortran/trans.h.jj	2019-11-11 21:04:05.210259346 +0100
> +++ gcc/fortran/trans.h	2019-12-05 11:23:55.935237355 +0100
> @@ -981,7 +981,6 @@ struct GTY(())	lang_type	 {
>     tree offset;
>     tree dtype;
>     tree dataptr_type;
> -  tree span;
>     tree base_decl[2];
>     tree nonrestricted_type;
>     tree caf_token;
> @@ -997,7 +996,6 @@ struct GTY(()) lang_decl {
>        address of target label.  */
>     tree stringlen;
>     tree addr;
> -  tree span;
>     /* For assumed-shape coarrays.  */
>     tree token, caf_offset;
>     unsigned int scalar_allocatable : 1;
> @@ -1008,7 +1006,6 @@ struct GTY(()) lang_decl {
>   
>   #define GFC_DECL_ASSIGN_ADDR(node) DECL_LANG_SPECIFIC(node)->addr
>   #define GFC_DECL_STRING_LEN(node) DECL_LANG_SPECIFIC(node)->stringlen
> -#define GFC_DECL_SPAN(node) DECL_LANG_SPECIFIC(node)->span
>   #define GFC_DECL_TOKEN(node) DECL_LANG_SPECIFIC(node)->token
>   #define GFC_DECL_CAF_OFFSET(node) DECL_LANG_SPECIFIC(node)->caf_offset
>   #define GFC_DECL_SAVED_DESCRIPTOR(node) \
> @@ -1059,7 +1056,6 @@ struct GTY(()) lang_decl {
>   #define GFC_TYPE_ARRAY_DTYPE(node) (TYPE_LANG_SPECIFIC(node)->dtype)
>   #define GFC_TYPE_ARRAY_DATAPTR_TYPE(node) \
>     (TYPE_LANG_SPECIFIC(node)->dataptr_type)
> -#define GFC_TYPE_ARRAY_SPAN(node) (TYPE_LANG_SPECIFIC(node)->span)
>   #define GFC_TYPE_ARRAY_BASE_DECL(node, internal) \
>     (TYPE_LANG_SPECIFIC(node)->base_decl[(internal)])
>   
> --- gcc/fortran/trans-array.h.jj	2019-09-26 22:02:40.236457478 +0200
> +++ gcc/fortran/trans-array.h	2019-12-05 11:07:52.255125510 +0100
> @@ -163,7 +163,7 @@ void gfc_trans_array_cobounds (tree, stm
>   
>   /* Build expressions for accessing components of an array descriptor.  */
>   void gfc_get_descriptor_offsets_for_info (const_tree, tree *, tree *, tree *, tree *,
> -					  tree *, tree *, tree *);
> +					  tree *, tree *, tree *, tree *);
>   
>   tree gfc_conv_descriptor_data_get (tree);
>   tree gfc_conv_descriptor_data_addr (tree);
> --- gcc/fortran/trans-array.c.jj	2019-11-01 09:01:50.111998535 +0100
> +++ gcc/fortran/trans-array.c	2019-12-05 11:09:38.545483494 +0100
> @@ -540,9 +540,10 @@ gfc_conv_shift_descriptor_lbound (stmtbl
>   
>   void
>   gfc_get_descriptor_offsets_for_info (const_tree desc_type, tree *data_off,
> -				     tree *dtype_off, tree *dim_off,
> -				     tree *dim_size, tree *stride_suboff,
> -				     tree *lower_suboff, tree *upper_suboff)
> +				     tree *dtype_off, tree *span_off,
> +				     tree *dim_off, tree *dim_size,
> +				     tree *stride_suboff, tree *lower_suboff,
> +				     tree *upper_suboff)
>   {
>     tree field;
>     tree type;
> @@ -552,6 +553,8 @@ gfc_get_descriptor_offsets_for_info (con
>     *data_off = byte_position (field);
>     field = gfc_advance_chain (TYPE_FIELDS (type), DTYPE_FIELD);
>     *dtype_off = byte_position (field);
> +  field = gfc_advance_chain (TYPE_FIELDS (type), SPAN_FIELD);
> +  *span_off = byte_position (field);
>     field = gfc_advance_chain (TYPE_FIELDS (type), DIMENSION_FIELD);
>     *dim_off = byte_position (field);
>     type = TREE_TYPE (TREE_TYPE (field));
> --- gcc/fortran/trans-types.c.jj	2019-08-27 12:27:05.678497543 +0200
> +++ gcc/fortran/trans-types.c	2019-12-05 11:39:33.868747876 +0100
> @@ -3266,7 +3266,7 @@ gfc_get_array_descr_info (const_tree typ
>     int rank, dim;
>     bool indirect = false;
>     tree etype, ptype, t, base_decl;
> -  tree data_off, dim_off, dtype_off, dim_size, elem_size;
> +  tree data_off, span_off, dim_off, dtype_off, dim_size, elem_size;
>     tree lower_suboff, upper_suboff, stride_suboff;
>     tree dtype, field, rank_off;
>   
> @@ -3323,12 +3323,13 @@ gfc_get_array_descr_info (const_tree typ
>     if (indirect)
>       base_decl = build1 (INDIRECT_REF, ptype, base_decl);
>   
> -  elem_size = fold_convert (gfc_array_index_type, TYPE_SIZE_UNIT (etype));
> -
> -  gfc_get_descriptor_offsets_for_info (type, &data_off, &dtype_off, &dim_off,
> -				       &dim_size, &stride_suboff,
> +  gfc_get_descriptor_offsets_for_info (type, &data_off, &dtype_off, &span_off,
> +				       &dim_off, &dim_size, &stride_suboff,
>   				       &lower_suboff, &upper_suboff);
>   
> +  t = fold_build_pointer_plus (base_decl, span_off);
> +  elem_size = build1 (INDIRECT_REF, gfc_array_index_type, t);
> +
>     t = base_decl;
>     if (!integer_zerop (data_off))
>       t = fold_build_pointer_plus (t, data_off);