diff mbox series

PR fortran/88810 -- Code clean up

Message ID 20190612190037.GA48174@troutmask.apl.washington.edu
State New
Headers show
Series PR fortran/88810 -- Code clean up | expand

Commit Message

Steve Kargl June 12, 2019, 7 p.m. UTC 
This PR flags a section of confusing but correct code.
The attach patch rewrites the code to hopefully provide
some clarity.  It has lived my tree for around 6 months,
so has sufferred through numerous regression tests.
OK to commit?

2019-06-12  Steven G. Kargl  <kargl@gcc.gnu.org>

	PR fortran/88810
	* dependency.c (gfc_dep_resolver): Re-arrange code to make the logic
	a bit more transparent.  Fix 2 nearby formatting issues.

Comments

Paul Richard Thomas June 13, 2019, 5:14 a.m. UTC | #1 
Hi Steve,

As the original author of the confusing code, I want to thank you for
de-confusing it!

OK

Paul

On Wed, 12 Jun 2019 at 20:01, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> This PR flags a section of confusing but correct code.
> The attach patch rewrites the code to hopefully provide
> some clarity.  It has lived my tree for around 6 months,
> so has sufferred through numerous regression tests.
> OK to commit?
>
> 2019-06-12  Steven G. Kargl  <kargl@gcc.gnu.org>
>
>         PR fortran/88810
>         * dependency.c (gfc_dep_resolver): Re-arrange code to make the logic
>         a bit more transparent.  Fix 2 nearby formatting issues.
>
> --
> Steve
Steve Kargl June 13, 2019, 5:49 p.m. UTC | #2 
Committed revision 272254.

I'm sure that when you authored the code, it wasn't confusing at all.
diff mbox series

Patch

Index: gcc/fortran/dependency.c
===================================================================
--- gcc/fortran/dependency.c	(revision 268276)
+++ gcc/fortran/dependency.c	(working copy)
@@ -2141,7 +2141,7 @@  gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_re
 
 	  /* Index for the reverse array.  */
 	  m = -1;
-	  for (n=0; n < lref->u.ar.dimen; n++)
+	  for (n = 0; n < lref->u.ar.dimen; n++)
 	    {
 	      /* Handle dependency when either of array reference is vector
 		 subscript. There is no dependency if the vector indices
@@ -2163,7 +2163,8 @@  gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_re
 
 	      if (lref->u.ar.dimen_type[n] == DIMEN_RANGE
 		  && rref->u.ar.dimen_type[n] == DIMEN_RANGE)
-		this_dep = check_section_vs_section (&lref->u.ar, &rref->u.ar, n);
+		this_dep = check_section_vs_section (&lref->u.ar,
+						     &rref->u.ar, n);
 	      else if (lref->u.ar.dimen_type[n] == DIMEN_ELEMENT
 		       && rref->u.ar.dimen_type[n] == DIMEN_RANGE)
 		this_dep = gfc_check_element_vs_section (lref, rref, n);
@@ -2196,35 +2197,38 @@  gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_re
 	      if (rref->u.ar.dimen_type[n] == DIMEN_RANGE
 		    && lref->u.ar.dimen_type[n] == DIMEN_RANGE)
 		{
-		  /* Set reverse if backward dependence and not inhibited.  */
-		  if (reverse && reverse[m] == GFC_ENABLE_REVERSE)
-		    reverse[m] = (this_dep == GFC_DEP_BACKWARD) ?
-			         GFC_REVERSE_SET : reverse[m];
+		  if (reverse)
+		    {
+		      /* Reverse if backward dependence and not inhibited.  */
+		      if (reverse[m] == GFC_ENABLE_REVERSE
+			  && this_dep == GFC_DEP_BACKWARD)
+			reverse[m] = GFC_REVERSE_SET;
 
-		  /* Set forward if forward dependence and not inhibited.  */
-		  if (reverse && reverse[m] == GFC_ENABLE_REVERSE)
-		    reverse[m] = (this_dep == GFC_DEP_FORWARD) ?
-			         GFC_FORWARD_SET : reverse[m];
+		      /* Forward if forward dependence and not inhibited.  */
+		      if (reverse[m] == GFC_ENABLE_REVERSE
+			  && this_dep == GFC_DEP_FORWARD)
+			reverse[m] = GFC_FORWARD_SET;
 
-		  /* Flag up overlap if dependence not compatible with
-		     the overall state of the expression.  */
-		  if (reverse && reverse[m] == GFC_REVERSE_SET
-		        && this_dep == GFC_DEP_FORWARD)
-		    {
-	              reverse[m] = GFC_INHIBIT_REVERSE;
-		      this_dep = GFC_DEP_OVERLAP;
+		      /* Flag up overlap if dependence not compatible with
+			 the overall state of the expression.  */
+		      if (reverse[m] == GFC_REVERSE_SET
+			  && this_dep == GFC_DEP_FORWARD)
+			{
+			  reverse[m] = GFC_INHIBIT_REVERSE;
+			  this_dep = GFC_DEP_OVERLAP;
+			}
+		      else if (reverse[m] == GFC_FORWARD_SET
+			       && this_dep == GFC_DEP_BACKWARD)
+			{
+			  reverse[m] = GFC_INHIBIT_REVERSE;
+			  this_dep = GFC_DEP_OVERLAP;
+			}
 		    }
-		  else if (reverse && reverse[m] == GFC_FORWARD_SET
-		        && this_dep == GFC_DEP_BACKWARD)
-		    {
-	              reverse[m] = GFC_INHIBIT_REVERSE;
-		      this_dep = GFC_DEP_OVERLAP;
-		    }
 
 		  /* If no intention of reversing or reversing is explicitly
 		     inhibited, convert backward dependence to overlap.  */
-		  if ((reverse == NULL && this_dep == GFC_DEP_BACKWARD)
-		      || (reverse != NULL && reverse[m] == GFC_INHIBIT_REVERSE))
+		  if ((!reverse && this_dep == GFC_DEP_BACKWARD)
+		      || (reverse && reverse[m] == GFC_INHIBIT_REVERSE))
 		    this_dep = GFC_DEP_OVERLAP;
 		}