diff mbox

[Fortran] Fix PR 71902

Message ID 4453cc4b-bc02-70bf-8328-7545a92a2a89@netcologne.de
State New
Headers show

Commit Message

Thomas Koenig Aug. 18, 2016, 7:24 p.m. UTC 
Hello Mikael,

> This doesn't look correct, what about substrings following component or
> array references?

You're right; corrected in the attached patch.  I have also added a test
case for this.

> PS: What about the original fix, wasn't it a dependency
> problem/inaccuracy after all?

The main error was using the allocatable attribute on the symbol that
we were dealing with a deferred length.  Changing the test to check for
expr->ts.deferred is the right thing to do.

There are still some some overly pessimistic assumptions in dependency
handling.  However, I am not sure that this causes actual performance
issues.

Regards

	Thomas

2016-08-18  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/71902
         * frontend-passes.c (realloc_string_callback):  Check for deferred
         on the expression instead for allocatable on the symbol.  Name 
temporary
         variable "realloc_string".

2016-08-18  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/71902
         * gfortran.dg/dependency_47.f90:  New test.
         * gfortran.dg/dependency_48.f90:  New test.

Comments

Mikael Morin Aug. 21, 2016, 2:39 p.m. UTC | #1 
Le 18/08/2016 à 21:24, Thomas Koenig a écrit :
> Hello Mikael,
>
>> This doesn't look correct, what about substrings following component or
>> array references?
>
> You're right; corrected in the attached patch.  I have also added a test
> case for this.
>
>> PS: What about the original fix, wasn't it a dependency
>> problem/inaccuracy after all?
>
> The main error was using the allocatable attribute on the symbol that
> we were dealing with a deferred length.  Changing the test to check for
> expr->ts.deferred is the right thing to do.
>
Seems sensible.  However deferred length includes pointers as well, so I 
would still keep additionally the original test for allocatable-ness. 
Or even better replace it with gfc_expr_attr(expr1).allocatable.
OK with that change.
Thanks

Mikael
diff mbox

Patch

Index: frontend-passes.c
===================================================================
--- frontend-passes.c	(Revision 239488)
+++ frontend-passes.c	(Arbeitskopie)
@@ -164,6 +164,8 @@  realloc_string_callback (gfc_code **c, int *walk_s
   gfc_expr *expr1, *expr2;
   gfc_code *co = *c;
   gfc_expr *n;
+  gfc_ref *ref;
+  bool found_substr;
 
   if (co->op != EXEC_ASSIGN)
     return 0;
@@ -170,7 +172,7 @@  realloc_string_callback (gfc_code **c, int *walk_s
 
   expr1 = co->expr1;
   if (expr1->ts.type != BT_CHARACTER || expr1->rank != 0
-      || !expr1->symtree->n.sym->attr.allocatable)
+      || !expr1->ts.deferred)
     return 0;
 
   expr2 = gfc_discard_nops (co->expr2);
@@ -177,6 +179,18 @@  realloc_string_callback (gfc_code **c, int *walk_s
   if (expr2->expr_type != EXPR_VARIABLE)
     return 0;
 
+  found_substr = false;
+  for (ref = expr2->ref; ref; ref = ref->next)
+    {
+      if (ref->type == REF_SUBSTRING)
+	{
+	  found_substr = true;
+	  break;
+	}
+    }
+  if (!found_substr)
+    return 0;
+
   if (!gfc_check_dependency (expr1, expr2, true))
     return 0;
 
@@ -190,7 +204,7 @@  realloc_string_callback (gfc_code **c, int *walk_s
   current_code = c;
   inserted_block = NULL;
   changed_statement = NULL;
-  n = create_var (expr2, "trim");
+  n = create_var (expr2, "realloc_string");
   co->expr2 = n;
   return 0;
 }