diff mbox

[fortran] PR 46065 - optimize trim

Message ID 1293477584.3714.24.camel@linux-fd1f.site
State New
Headers show

Commit Message

Thomas Koenig Dec. 27, 2010, 7:19 p.m. UTC
Hello world,

the attached patch changes trim(a) into a(1:len_trim(a)), thus
eliminating the need for a temporary.

The patch is rather straightforward, and should be rather safe.  This is
a bit of speed and memory use improvement that I would still like to see
in 4.6.

Regression-tested.

OK for trunk?

	Thomas

2010-12-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/47065
	* frontend-passes.c (optimize_trim):  New function.
	(optimize_expr):  Call it.

2010-12-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/47065
	* gfortran.dg/optimize_trim_3.f90:  New test.

Comments

Thomas Koenig Dec. 27, 2010, 8:22 p.m. UTC | #1
I forgot the test case, here it is.
Tobias Burnus Dec. 29, 2010, 10:41 a.m. UTC | #2
Hello Thomas,

Thomas Koenig wrote:
> the attached patch changes trim(a) into a(1:len_trim(a)), thus
> eliminating the need for a temporary.

Sounds great - I expect that the main use will be I/O transfer 
statements, for which the transformation is save. However, I worry that 
the following program is mishandled, "sub" is the obvious case while 
"two" is a special case, cf. PR 46896.

Tobias

character(len=12) :: str
str = '1234567890'
call sub(trim(str), str)
! Should print '12345       '
print *, str
call two(trim(str))
! Should print '123         '
print *, str
contains
subroutine sub(a,b)
   character(len=*), intent(in) :: a
   character(len=*), intent(out) :: b
   b = ''
   b = a(1:5)
end subroutine
subroutine two(a)
   character(len=*), intent(in) :: a
   str = ''
   str(1:3) = a(1:3)
end subroutine two
end
Thomas Koenig Dec. 29, 2010, 12:28 p.m. UTC | #3
Hello Tobias,

> Hello Thomas,
> 
> Thomas Koenig wrote:
> > the attached patch changes trim(a) into a(1:len_trim(a)), thus
> > eliminating the need for a temporary.
> 
> Sounds great - I expect that the main use will be I/O transfer 
> statements, for which the transformation is save. However, I worry that 
> the following program is mishandled, "sub" is the obvious case while 
> "two" is a special case, cf. PR 46896.

Your concern is valid.  I checked your test case, and to my surprise
found that it doesn't cause the expected problems.  The reason for that
is that the code walker currently doesn't handle argument lists for
subroutine calls. Ouch.

I'll put this patch on the back burner and address the subroutine call
issue first.

Thanks for your comments!

	Thomas
diff mbox

Patch

Index: frontend-passes.c
===================================================================
--- frontend-passes.c	(Revision 168201)
+++ frontend-passes.c	(Arbeitskopie)
@@ -34,6 +34,7 @@  static void optimize_namespace (gfc_namespace *);
 static void optimize_assignment (gfc_code *);
 static bool optimize_op (gfc_expr *);
 static bool optimize_comparison (gfc_expr *, gfc_intrinsic_op);
+static bool optimize_trim (gfc_expr *);
 
 /* Entry point - run all passes for a namespace.  So far, only an
    optimization pass is run.  */
@@ -68,6 +69,10 @@  static int
 optimize_expr (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
 	       void *data ATTRIBUTE_UNUSED)
 {
+
+  if (optimize_trim (*e))
+    gfc_simplify_expr (*e, 0);
+
   if ((*e)->expr_type == EXPR_OP && optimize_op (*e))
     gfc_simplify_expr (*e, 0);
   return 0;
@@ -395,6 +400,70 @@  optimize_comparison (gfc_expr *e, gfc_intrinsic_op
   return false;
 }
 
+/* Optimize a trim function by replacing it with an equivalent substring
+   involving a call to len_trim.  This only works for expressions where
+   variables are trimmed.  Return true if anything was modified.  */
+
+static bool
+optimize_trim (gfc_expr *e)
+{
+  gfc_expr *a;
+  gfc_ref *ref;
+  gfc_expr *fcn;
+  gfc_actual_arglist *actual_arglist, *next;
+
+  if (e->ts.type != BT_CHARACTER || e->expr_type != EXPR_FUNCTION
+      || e->value.function.isym == NULL
+      || e->value.function.isym->id != GFC_ISYM_TRIM)
+    return false;
+
+  a = e->value.function.actual->expr;
+
+  if (a->expr_type != EXPR_VARIABLE)
+    return false;
+
+  if (a->ref)
+    {
+      /* FIXME - also handle substring references, by modifying the
+	 reference itself.  Make sure not to evaluate functions in
+	 the references twice.  */
+      return false;
+    }
+  else
+    {
+      strip_function_call (e);
+
+      /* Create the reference.  */
+
+      ref = gfc_get_ref ();
+      ref->type = REF_SUBSTRING;
+
+      /* Set the start of the reference.  */
+
+      ref->u.ss.start = gfc_get_int_expr (gfc_default_integer_kind, NULL, 1);
+
+      /* Build the function call to len_trim(x, gfc_defaul_integer_kind).  */
+
+      fcn = gfc_get_expr ();
+      fcn->expr_type = EXPR_FUNCTION;
+      fcn->value.function.isym =
+	gfc_intrinsic_function_by_id (GFC_ISYM_LEN_TRIM);
+      actual_arglist = gfc_get_actual_arglist ();
+      actual_arglist->expr = gfc_copy_expr (e);
+      next = gfc_get_actual_arglist ();
+      next->expr = gfc_get_int_expr (gfc_default_integer_kind, NULL,
+				     gfc_default_integer_kind);
+      actual_arglist->next = next;
+      fcn->value.function.actual = actual_arglist;
+
+      /* Set the end of the reference to the call to len_trim.  */
+
+      ref->u.ss.end = fcn;
+      e->ref = ref;
+      return true;
+    }
+}
+
 #define WALK_SUBEXPR(NODE) \
   do							\
     {							\