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
Related show

Commit Message

Steve Kargl June 12, 2019, 7 p.m.
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

Comments

Paul Richard Thomas June 13, 2019, 5:14 a.m. | #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




-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Steve Kargl June 13, 2019, 5:49 p.m. | #2
Committed revision 272254.

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

-- 
steve


On Thu, Jun 13, 2019 at 06:14:59AM +0100, Paul Richard Thomas wrote:
> 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

> 

> 

> 

> -- 

> "If you can't explain it simply, you don't understand it well enough"

> - Albert Einstein


-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

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;
 		}