Patches to improve Fortran support

Patches to improve Fortran support

[David Lecomber]

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

I have attached one of the wrong patch files to my email of Jan 13.

The first attached patch only cured the tendency of gdb to allocate -ve large
amounts of memory, which was my first attempt at curing the fortran array problem.
That was more curing the symptom than the cause, and doesn't work fully anyway..

The new attachment is the fix to the source of the problem.  Apologies for the 
confusion.

Cheers
David

[-- Attachment #2: gdb.array-ptr-patch --]
[-- Type: text/plain, Size: 1291 bytes --]

*** ../../TMP/gdb-5.3/gdb/eval.c	Sun Dec 15 22:29:59 2002
--- eval.c	Sun Dec 15 22:28:41 2002
*************** evaluate_subexp_standard (struct type *e
*** 1383,1392 ****
  	  offset_item =
  	    array_size_array[i] * offset_item + subscript_array[i];
  
- 	/* Construct a value node with the value of the offset */
- 
- 	arg2 = value_from_longest (builtin_type_f_integer, offset_item);
- 
  	/* Let us now play a dirty trick: we will take arg1 
  	   which is a value node pointing to the topmost level
  	   of the multidimensional array-set and pretend
--- 1383,1388 ----
*************** evaluate_subexp_standard (struct type *e
*** 1395,1401 ****
  	   returns the correct type value */
  
  	VALUE_TYPE (arg1) = tmp_type;
! 	return value_ind (value_add (value_coerce_array (arg1), arg2));
        }
  
      case BINOP_LOGICAL_AND:
--- 1391,1405 ----
  	   returns the correct type value */
  	
  	VALUE_TYPE (arg1) = tmp_type;
! 
! 	f77_get_dynamic_lowerbound (tmp_type, &lower);
! 
! 	/* Construct a value node with the value of the offset */
! 	/* lower will get subtracted off in value_subscript, hence add it here */
! 
! 	arg2 = value_from_longest (builtin_type_f_integer, offset_item + lower);
! 
! 	return value_subscript(arg1, arg2);
        }
  
      case BINOP_LOGICAL_AND:

[PATCH] Improvements to Fortran support

[David Lecomber]

Reposting as I now have the copyright assignment in place..

Fortran arrays are presently allocated in their entirety and then the
correct element is pulled out.  This (a) doesn't scale, (b) doesn't
work if the array is a parameter to a subroutine and supplied with a
(*) in the dimensions (GDB runs out of memory doing malloc( -1 )..)

The fix here makes the behaviour the same as the code for doing C
arrays..


*** eval.c	Sun Dec 15 22:29:59 2002
--- eval.c	Sun Dec 15 22:28:41 2002
*************** evaluate_subexp_standard (struct type *e
*** 1383,1392 ****
  	  offset_item =
  	    array_size_array[i] * offset_item + subscript_array[i];
  
- 	/* Construct a value node with the value of the offset */
- 
- 	arg2 = value_from_longest (builtin_type_f_integer, offset_item);
- 
  	/* Let us now play a dirty trick: we will take arg1 
  	   which is a value node pointing to the topmost level
  	   of the multidimensional array-set and pretend
--- 1383,1388 ----
*************** evaluate_subexp_standard (struct type *e
*** 1395,1401 ****
  	   returns the correct type value */
  
  	VALUE_TYPE (arg1) = tmp_type;
! 	return value_ind (value_add (value_coerce_array (arg1), arg2));
        }
  
      case BINOP_LOGICAL_AND:
--- 1391,1405 ----
  	   returns the correct type value */
  	
  	VALUE_TYPE (arg1) = tmp_type;
! 
! 	f77_get_dynamic_lowerbound (tmp_type, &lower);
! 
! 	/* Construct a value node with the value of the offset */
! 	/* lower will get subtracted off in value_subscript, hence add it here */
! 
! 	arg2 = value_from_longest (builtin_type_f_integer, offset_item + lower);
! 
! 	return value_subscript(arg1, arg2);
        }
  
      case BINOP_LOGICAL_AND:


David

Re: [PATCH] Improvements to Fortran support

[Daniel Jacobowitz]

On Fri, May 16, 2003 at 12:21:03PM +0100, David Lecomber wrote:
> Reposting as I now have the copyright assignment in place..
> 
> Fortran arrays are presently allocated in their entirety and then the
> correct element is pulled out.  This (a) doesn't scale, (b) doesn't
> work if the array is a parameter to a subroutine and supplied with a
> (*) in the dimensions (GDB runs out of memory doing malloc( -1 )..)
> 
> The fix here makes the behaviour the same as the code for doing C
> arrays..

Thanks for the patch and your patience.  I have a couple of small
comments on your changes.

First of all, when submitting a patch, please include a standard
ChangeLog entry - even for small patches like this one, it makes it
easier to see at a glance what's going on and where.  For this patch it
would look something like:

2003-05-16  David Lecomber  <david@streamline-computing.com>

	* eval.c (evaluate_subexp_standard): Correct handling
	for Fortran multidimensional array subscripts.

Secondly, some nits about formatting:
> ! 	/* Construct a value node with the value of the offset */
> ! 	/* lower will get subtracted off in value_subscript, hence add it here */

Should be:
	/* Construct a value node with the value of the offset.  LOWER
	   will get subtracted off in value_subscript, hence add it
	   here.  */

(i.e. full sentences, two spaces after a full stop.)


And, my only substantive comments:

 - Could you explain why this makes a difference?  You change value_ind
   (value_add (value_coerce_array (), idx))) into value_subscript (array,
   idx).  value_subscript will call value_coerce_array, then value_add and
   value_ind on the result.

 - How does f77_get_dynamic_lowerbound end up being called from
   value_subscript?  I couldn't figure it out, which says that either
   I woke up denser than usual this morning or a better comment is in
   order.

> *** eval.c	Sun Dec 15 22:29:59 2002
> --- eval.c	Sun Dec 15 22:28:41 2002
> *************** evaluate_subexp_standard (struct type *e
> *** 1383,1392 ****
>   	  offset_item =
>   	    array_size_array[i] * offset_item + subscript_array[i];
>   
> - 	/* Construct a value node with the value of the offset */
> - 
> - 	arg2 = value_from_longest (builtin_type_f_integer, offset_item);
> - 
>   	/* Let us now play a dirty trick: we will take arg1 
>   	   which is a value node pointing to the topmost level
>   	   of the multidimensional array-set and pretend
> --- 1383,1388 ----
> *************** evaluate_subexp_standard (struct type *e
> *** 1395,1401 ****
>   	   returns the correct type value */
>   
>   	VALUE_TYPE (arg1) = tmp_type;
> ! 	return value_ind (value_add (value_coerce_array (arg1), arg2));
>         }
>   
>       case BINOP_LOGICAL_AND:
> --- 1391,1405 ----
>   	   returns the correct type value */
>   	
>   	VALUE_TYPE (arg1) = tmp_type;
> ! 
> ! 	f77_get_dynamic_lowerbound (tmp_type, &lower);
> ! 
> ! 	/* Construct a value node with the value of the offset */
> ! 	/* lower will get subtracted off in value_subscript, hence add it here */
> ! 
> ! 	arg2 = value_from_longest (builtin_type_f_integer, offset_item + lower);
> ! 
> ! 	return value_subscript(arg1, arg2);
>         }
>   
>       case BINOP_LOGICAL_AND:
> 
> 
> David
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Improvements to Fortran support

[David Carlton]

On Fri, 16 May 2003 12:21:03 +0100, David Lecomber <david@streamline-computing.com> said:

[ Fortran array fix. ]

I don't have any comments on the fix itself, but could you perhaps add
some tests for this problem as well?  Our Fortran testsuite is pretty
pathetic, and we have to start somewhere...

David Carlton
carlton@math.stanford.edu

[PATCH] Large array fix for Fortran.

[David Lecomber]

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

Resubmitted as I now have copyright assignment..

This patch limits the number of elements printed from a
multidimensional array to the correct number of elements, rather than
the number of top level elements (eg. rows).  Very useful for most
Fortran programmers, as they have a tendency to have obscenely large
multi-d arrays and without this fix info locals, info args and others
tend to be kinda slow..

A similar fix for C wouldn't go amiss, but is less urgent..

David

[-- Attachment #2: gdb.huge-array-patch --]
[-- Type: text/plain, Size: 4228 bytes --]

*** f-valprint.c	Wed Mar  7 02:57:08 2001
--- f-valprint.c	Mon Oct  7 10:22:57 2002
*************** static void f77_print_array (struct type
*** 46,52 ****
  			     enum val_prettyprint);
  static void f77_print_array_1 (int, int, struct type *, char *,
  			       CORE_ADDR, struct ui_file *, int, int, int,
! 			       enum val_prettyprint);
  static void f77_create_arrayprint_offset_tbl (struct type *,
  					      struct ui_file *);
  static void f77_get_dynamic_length_of_aggregate (struct type *);
--- 46,53 ----
  			     enum val_prettyprint);
  static void f77_print_array_1 (int, int, struct type *, char *,
  			       CORE_ADDR, struct ui_file *, int, int, int,
! 			       enum val_prettyprint,
! 			       int *elts);
  static void f77_create_arrayprint_offset_tbl (struct type *,
  					      struct ui_file *);
  static void f77_get_dynamic_length_of_aggregate (struct type *);
*************** f77_create_arrayprint_offset_tbl (struct
*** 270,300 ****
      }
  }
  
  /* Actual function which prints out F77 arrays, Valaddr == address in 
     the superior.  Address == the address in the inferior.  */
- 
  static void
  f77_print_array_1 (int nss, int ndimensions, struct type *type, char *valaddr,
  		   CORE_ADDR address, struct ui_file *stream, int format,
! 		   int deref_ref, int recurse, enum val_prettyprint pretty)
  {
    int i;
  
    if (nss != ndimensions)
      {
!       for (i = 0; i < F77_DIM_SIZE (nss); i++)
  	{
  	  fprintf_filtered (stream, "( ");
  	  f77_print_array_1 (nss + 1, ndimensions, TYPE_TARGET_TYPE (type),
  			     valaddr + i * F77_DIM_OFFSET (nss),
  			     address + i * F77_DIM_OFFSET (nss),
! 			     stream, format, deref_ref, recurse, pretty);
  	  fprintf_filtered (stream, ") ");
  	}
      }
    else
      {
!       for (i = 0; (i < F77_DIM_SIZE (nss) && i < print_max); i++)
  	{
  	  val_print (TYPE_TARGET_TYPE (type),
  		     valaddr + i * F77_DIM_OFFSET (ndimensions),
--- 271,306 ----
      }
  }
  
+ 
+ 
  /* Actual function which prints out F77 arrays, Valaddr == address in 
     the superior.  Address == the address in the inferior.  */
  static void
  f77_print_array_1 (int nss, int ndimensions, struct type *type, char *valaddr,
  		   CORE_ADDR address, struct ui_file *stream, int format,
! 		   int deref_ref, int recurse, enum val_prettyprint pretty,
! 		   int *elts)
  {
    int i;
  
    if (nss != ndimensions)
      {
!       for (i = 0; i < F77_DIM_SIZE (nss)  && *elts < print_max; i++)
  	{
  	  fprintf_filtered (stream, "( ");
  	  f77_print_array_1 (nss + 1, ndimensions, TYPE_TARGET_TYPE (type),
  			     valaddr + i * F77_DIM_OFFSET (nss),
  			     address + i * F77_DIM_OFFSET (nss),
! 			     stream, format, deref_ref, recurse, pretty, elts);
  	  fprintf_filtered (stream, ") ");
  	}
+       if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
+ 	fprintf_filtered (stream, "...");
+       }
      }
    else
      {
!       for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++ , (*elts)++)
  	{
  	  val_print (TYPE_TARGET_TYPE (type),
  		     valaddr + i * F77_DIM_OFFSET (ndimensions),
*************** f77_print_array_1 (int nss, int ndimensi
*** 305,311 ****
  	  if (i != (F77_DIM_SIZE (nss) - 1))
  	    fprintf_filtered (stream, ", ");
  
! 	  if (i == print_max - 1)
  	    fprintf_filtered (stream, "...");
  	}
      }
--- 311,317 ----
  	  if (i != (F77_DIM_SIZE (nss) - 1))
  	    fprintf_filtered (stream, ", ");
  
! 	  if (( *elts) > print_max - 1)
  	    fprintf_filtered (stream, "...");
  	}
      }
*************** f77_print_array (struct type *type, char
*** 320,325 ****
--- 326,332 ----
  		 enum val_prettyprint pretty)
  {
    int ndimensions;
+   int elts = 0;
  
    ndimensions = calc_f77_array_dims (type);
  
*************** f77_print_array (struct type *type, char
*** 334,340 ****
    f77_create_arrayprint_offset_tbl (type, stream);
  
    f77_print_array_1 (1, ndimensions, type, valaddr, address, stream, format,
! 		     deref_ref, recurse, pretty);
  }
  \f
  
--- 341,347 ----
    f77_create_arrayprint_offset_tbl (type, stream);
  
    f77_print_array_1 (1, ndimensions, type, valaddr, address, stream, format,
! 		     deref_ref, recurse, pretty, &elts);
  }
  \f
  

Re: [PATCH] Large array fix for Fortran.

[Daniel Jacobowitz]

On Fri, May 16, 2003 at 12:24:26PM +0100, David Lecomber wrote:
> Resubmitted as I now have copyright assignment..
> 
> This patch limits the number of elements printed from a
> multidimensional array to the correct number of elements, rather than
> the number of top level elements (eg. rows).  Very useful for most
> Fortran programmers, as they have a tendency to have obscenely large
> multi-d arrays and without this fix info locals, info args and others
> tend to be kinda slow..
> 
> A similar fix for C wouldn't go amiss, but is less urgent..
> 
> David

Some comments:

  - Missing ChangeLog.
  - You introduced a whitespace change on the before the body of
    f77_print_array_1; please try to avoid mixing whitespace changes
    with bug fixes.  Whitespace in your changes is a little bit off,
    too, for instance the two spaces before the &&:
> !       for (i = 0; i < F77_DIM_SIZE (nss)  && *elts < print_max; i++)
    and the space before the comma:
> !       for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++ , (*elts)++)
    It may be trivial, but consistency in this sort of thing makes the
    code much easier to read.

Otherwise, this patch is OK.  I'd appreciate it if you could repost it
with ChangeLog and formatting fixes, though.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer