public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
To: Jerry DeLisle <jvdelisle@charter.net>
Cc: "Nicolas Koenig" <koenigni@student.ethz.ch>,
	"Dominique d'Humières" <dominiq@lps.ens.fr>,
	gfortran <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch, fortran] PR35339 Optimize implied do loops in io statements
Date: Sat, 03 Jun 2017 18:12:00 -0000	[thread overview]
Message-ID: <20170603181224.bhfvgl6dm5brevpr@nbbrfq.cc.univie.ac.at> (raw)
In-Reply-To: <5e1bd32c-de24-8f95-ecf2-0ab76abb6092@charter.net>

On Sat, Jun 03, 2017 at 09:25:31AM -0700, Jerry DeLisle wrote:
> On 06/03/2017 06:48 AM, Nicolas Koenig wrote:
> > Hello everyone,
> > 
> > here is a version of the patch that includes a workaround for PR 80960. I have
> > also included a separate test case for the failure that Dominique detected. The
> > style issues should be fixed.
> > 
> > Regression-tested. OK for trunk?
> > 
> 
> Yes, OK.

There still are plenty of coding-style issues (see below).
Can you please rectify them before committing?

Also you change gfc-internals.texi without a ChangeLog entry. I guess
this was an accident?

thanks,

$ contrib/check_GNU_style.sh /tmp/p9.diff 

Blocks of 8 spaces should be replaced with tabs.
40:+        break;
55:+        return false;
61:+        {
64:+              curr->block->next = NULL;
65:+              gfc_free_statements(curr);
70:+        }
92:+          || ref->u.ar.dimen_type[i] != DIMEN_ELEMENT)
93:+        return false;
98:+        {
111:+              iters[i] = stack_top->iter;
116:+        case EXPR_CONSTANT:
120:+          switch (start->value.op.op)
125:+	        std::swap(start->value.op.op1, start->value.op.op2);
130:+	          || start->value.op.op1->ref)
131:+	        return false;
132:+              if (!stack_top || !stack_top->iter 
135:+	        return false;
146:+        }
160:+        continue;
163:+        {
174:+          break;
214:+        {
215:+          curr->next = prev->next->next;
216:+          prev->next = curr;
219:+        {
220:+          curr->next = stack_top->code->block->next->next->next;
253:+        {
254:+          first.prev = &write;
260:+        }

Trailing whitespace.
18:+   
20:+     
22:+     
25:+static bool 
28:+  gfc_code *curr; 
44:+       
94:+      
106:+	  if (!stack_top || !stack_top->iter 
108:+	    iters[i] = NULL; 
128:+	      if ((start->value.op.op1->expr_type!= EXPR_VARIABLE 
132:+              if (!stack_top || !stack_top->iter 
133:+		  || stack_top->iter->var->symtree 
136:+	      iters[i] = stack_top->iter; 
152:+  new_e->rank = future_rank; 
176:+	  new_e->ref->u.ar.dimen_type[i] = DIMEN_RANGE; 
218:+      else 
244:+  
249:+  

Dot, space, space, new sentence.
17:+   optimize by replacing do loops with their analog array slices. For example:

There should be exactly one space between function name and parenthesis.
26:+traverse_io_block(gfc_code *code, bool *has_reached, gfc_code *prev)
60:+      if (traverse_io_block(curr->block->next, has_reached, prev))
65:+              gfc_free_statements(curr);
74:+  gcc_assert(curr->op == EXEC_TRANSFER);
96:+      gfc_simplify_expr(start, 0);
125:+	        std::swap(start->value.op.op1, start->value.op.op2);
126:+	      gcc_fallthrough();
150:+  new_e = gfc_copy_expr(curr->expr1);
154:+    new_e->shape = gfc_get_shape(new_e->rank);
165:+	  gfc_internal_error("bad expression");
170:+	  gfc_free_expr(new_e->ref->u.ar.start[i]);
171:+	  new_e->ref->u.ar.start[i] = gfc_copy_expr(iters[i]->start);
172:+	  new_e->ref->u.ar.end[i] = gfc_copy_expr(iters[i]->end);
173:+	  new_e->ref->u.ar.stride[i] = gfc_copy_expr(iters[i]->step);
178:+	  gfc_free_expr(new_e->ref->u.ar.start[i]);
179:+	  expr = gfc_copy_expr(start);
180:+	  expr->value.op.op1 = gfc_copy_expr(iters[i]->start);
182:+	  gfc_simplify_expr(new_e->ref->u.ar.start[i], 0);
183:+	  expr = gfc_copy_expr(start);
184:+	  expr->value.op.op1 = gfc_copy_expr(iters[i]->end);
186:+	  gfc_simplify_expr(new_e->ref->u.ar.end[i], 0);
187:+	  switch(start->value.op.op)
191:+	      new_e->ref->u.ar.stride[i] = gfc_copy_expr(iters[i]->step);
194:+	      expr = gfc_copy_expr(start);
195:+	      expr->value.op.op1 = gfc_copy_expr(iters[i]->step);
197:+	      gfc_simplify_expr(new_e->ref->u.ar.stride[i], 0);
200:+	      gfc_internal_error("bad op");
204:+	  gfc_internal_error("bad expression");
258:+	  traverse_io_block((*curr)->block->next, &b, prev);

> 
> Thanks for the work.
> 
> Jerry

  reply	other threads:[~2017-06-03 18:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-29 15:49 Dominique d'Humières
2017-05-29 16:08 ` Nicolas Koenig
2017-05-29 16:34   ` Dominique d'Humières
2017-05-31  6:49   ` Bernhard Reutner-Fischer
2017-05-31 15:49     ` Dominique d'Humières
2017-05-31 16:04       ` Dominique d'Humières
2017-05-31 19:11         ` Nicolas Koenig
2017-05-31 23:34           ` Bernhard Reutner-Fischer
2017-06-01  9:31           ` Dominique d'Humières
2017-06-01 14:19             ` Dominique d'Humières
2017-06-01 14:37               ` Dominique d'Humières
2017-06-03 13:48                 ` Nicolas Koenig
2017-06-03 16:25                   ` Jerry DeLisle
2017-06-03 18:12                     ` Bernhard Reutner-Fischer [this message]
2017-06-05 20:39                     ` Nicolas Koenig
2017-06-06 11:05                       ` Markus Trippelsdorf
2017-06-07 15:13                         ` Renlin Li
2017-11-02  8:25                   ` [testsuite, committed] Fix scan pattern in gfortran.dg/implied_do_io_1.f90 Tom de Vries
  -- strict thread matches above, loose matches on Subject: below --
2017-05-27 21:47 [Patch, fortran] PR35339 Optimize implied do loops in io statements Nicolas Koenig
2017-05-28 22:09 ` Jerry DeLisle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170603181224.bhfvgl6dm5brevpr@nbbrfq.cc.univie.ac.at \
    --to=rep.dot.nop@gmail.com \
    --cc=dominiq@lps.ens.fr \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jvdelisle@charter.net \
    --cc=koenigni@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).