Patchwork [fortran,4/5] PR 45648: Inline transpose part 2: Remove ss lookup

login
register
mail settings
Submitter Mikael Morin
Date Sept. 20, 2010, 10:21 p.m.
Message ID <20100920222130.27269.77777@gimli.local>
Download mbox | patch
Permalink /patch/65264/
State New
Headers show

Comments

Mikael Morin - Sept. 20, 2010, 10:21 p.m.
This one is optional. 
I noticed while working on these patches that there were loops to get the ss corresponding to the expr to be evaluated. This is a contradiction with other places in the code where se->ss is supposed to be in sync (this is the point of gfc_advance_se_ss_chain) with the expression to be calculated (grep "ss->expr == expr"). Furthermore, the first half of the function was using secss (with lookup) and the second ss (without).
Thus, this removes the ss lookup and replaces the asserts with their equivalent without lookup. There is no behavioural change and this is not directly related to the patchset, so this is optional. 

OK for trunk?
2010-09-20  Mikael Morin  <mikael@gcc.gnu.org>

	* trans-array.c (gfc_conv_expr_descriptor): Remove ss lookup.
	Update asserts accordingly.
Paul Richard Thomas - Sept. 21, 2010, 1:58 p.m.
Mikael,

This is OK.

Cheers

Paul

On 9/21/10, Mikael Morin <mikael.morin@sfr.fr> wrote:
> This one is optional.
> I noticed while working on these patches that there were loops to get the ss
> corresponding to the expr to be evaluated. This is a contradiction with
> other places in the code where se->ss is supposed to be in sync (this is the
> point of gfc_advance_se_ss_chain) with the expression to be calculated (grep
> "ss->expr == expr"). Furthermore, the first half of the function was using
> secss (with lookup) and the second ss (without).
> Thus, this removes the ss lookup and replaces the asserts with their
> equivalent without lookup. There is no behavioural change and this is not
> directly related to the patchset, so this is optional.
>
> OK for trunk?
>
>
>

Patch

diff --git a/trans-array.c b/trans-array.c
index 9171183..947ed4b 100644
--- a/trans-array.c
+++ b/trans-array.c
@@ -5164,7 +5164,6 @@  void
 gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
 {
   gfc_loopinfo loop;
-  gfc_ss *secss;
   gfc_ss_info *info;
   int need_tmp;
   int n;
@@ -5177,6 +5176,7 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
   bool subref_array_target = false;
   gfc_expr *arg;
 
+  gcc_assert (ss != NULL);
   gcc_assert (ss != gfc_ss_terminator);
 
   /* Special case things we know we can pass easily.  */
@@ -5186,16 +5186,12 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
       /* If we have a linear array section, we can pass it directly.
 	 Otherwise we need to copy it into a temporary.  */
 
-      /* Find the SS for the array section.  */
-      secss = ss;
-      while (secss != gfc_ss_terminator && secss->type != GFC_SS_SECTION)
-	secss = secss->next;
-
-      gcc_assert (secss != gfc_ss_terminator);
-      info = &secss->data.info;
+      gcc_assert (ss->type == GFC_SS_SECTION);
+      gcc_assert (ss->expr == expr);
+      info = &ss->data.info;
 
       /* Get the descriptor for the array.  */
-      gfc_conv_ss_descriptor (&se->pre, secss, 0);
+      gfc_conv_ss_descriptor (&se->pre, ss, 0);
       desc = info->descriptor;
 
       subref_array_target = se->direct_byref && is_subref_array (expr);
@@ -5271,26 +5267,28 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
 	 array descriptor.  We still need to go through the scalarizer
 	 to create the descriptor.  Elemental functions ar handled as
 	 arbitrary expressions, i.e. copy to a temporary.  */
-      secss = ss;
-      /* Look for the SS for this function.  */
-      while (secss != gfc_ss_terminator
-	     && (secss->type != GFC_SS_FUNCTION || secss->expr != expr))
-      	secss = secss->next;
 
       if (se->direct_byref)
 	{
-	  gcc_assert (secss != gfc_ss_terminator);
+	  gcc_assert (ss->type == GFC_SS_FUNCTION && ss->expr == expr);
 
 	  /* For pointer assignments pass the descriptor directly.  */
-	  se->ss = secss;
+	  if (se->ss == NULL)
+	    se->ss = ss;
+	  else
+	    gcc_assert (se->ss == ss);
 	  se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
 	  gfc_conv_expr (se, expr);
 	  return;
 	}
 
-      if (secss == gfc_ss_terminator)
+      if (ss->expr != expr)
 	{
 	  /* Elemental function.  */
+	  gcc_assert ((expr->value.function.esym != NULL
+		       && expr->value.function.esym->attr.elemental)
+		      || (expr->value.function.isym != NULL
+			  && expr->value.function.isym->elemental));
 	  need_tmp = 1;
 	  if (expr->ts.type == BT_CHARACTER
 		&& expr->ts.u.cl->length->expr_type != EXPR_CONSTANT)
@@ -5301,7 +5299,7 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
       else
 	{
 	  /* Transformational function.  */
-	  info = &secss->data.info;
+	  info = &ss->data.info;
 	  need_tmp = 0;
 	}
       break;
@@ -5314,12 +5312,10 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
 	{
 	  need_tmp = 0;
 	  info = &ss->data.info;
-	  secss = ss;
 	}
       else
 	{
 	  need_tmp = 1;
-	  secss = NULL;
 	  info = NULL;
 	}
       break;
@@ -5327,7 +5323,6 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
     default:
       /* Something complicated.  Copy it into a temporary.  */
       need_tmp = 1;
-      secss = NULL;
       info = NULL;
       break;
     }
@@ -5443,7 +5438,6 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
 	se->string_length =  gfc_get_expr_charlen (expr);
 
       desc = info->descriptor;
-      gcc_assert (secss && secss != gfc_ss_terminator);
       if (se->direct_byref && !se->byref_noassign)
 	{
 	  /* For pointer assignments we fill in the destination.  */
@@ -5465,7 +5459,7 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
       /* The following can be somewhat confusing.  We have two
          descriptors, a new one and the original array.
          {parm, parmtype, dim} refer to the new one.
-         {desc, type, n, secss, loop} refer to the original, which maybe
+         {desc, type, n, loop} refer to the original, which maybe
          a descriptorless array.
          The bounds of the scalarization are the bounds of the section.
          We don't have to worry about numeric overflows when calculating