public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Mikael Morin <mikael.morin@sfr.fr>
To: gfortran <fortran@gcc.gnu.org> , GCC patches <gcc-patches@gcc.gnu.org>
Subject: [Patch, fortran] [57..59/66] inline sum and product: Prevent regressions: Fix {min, max}{loc, val}
Date: Thu, 27 Oct 2011 23:36:00 -0000	[thread overview]
Message-ID: <20111027233322.18581.12112@gimli.local> (raw)
In-Reply-To: <20111027233305.18581.72802@gimli.local>

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

Patches 58 and 59 fix the {min,max}loc and {min,max}val intrinsics which use
multiple loops. See the comments in the patches for details.
Patch 57 avoids duplicated offset calculation in the code generated.
OK?

[-- Attachment #2: pr43829-57.CL --]
[-- Type: text/plain, Size: 150 bytes --]

2011-10-19  Mikael Morin  <mikael@gcc.gnu.org>

	* trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Don't calculate
	offset twice in generated code.

[-- Attachment #3: pr43829-57.patch --]
[-- Type: text/x-diff, Size: 1999 bytes --]

diff --git a/trans-intrinsic.c b/trans-intrinsic.c
index c3a414b..ee162ea 100644
--- a/trans-intrinsic.c
+++ b/trans-intrinsic.c
@@ -3090,6 +3090,14 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
       TREE_USED (lab2) = 1;
     }
 
+  /* An offset must be added to the loop
+     counter to obtain the required position.  */
+  gcc_assert (loop.from[0]);
+
+  tmp = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
+			 gfc_index_one_node, loop.from[0]);
+  gfc_add_modify (&loop.pre, offset, tmp);
+
   gfc_mark_ss_chain_used (arrayss, 1);
   if (maskss)
     gfc_mark_ss_chain_used (maskss, 1);
@@ -3123,16 +3131,6 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
   /* Assign the value to the limit...  */
   gfc_add_modify (&ifblock, limit, arrayse.expr);
 
-  /* Remember where we are.  An offset must be added to the loop
-     counter to obtain the required position.  */
-  if (loop.from[0])
-    tmp = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
-			   gfc_index_one_node, loop.from[0]);
-  else
-    tmp = gfc_index_one_node;
-
-  gfc_add_modify (&block, offset, tmp);
-
   if (nonempty == NULL && HONOR_NANS (DECL_MODE (limit)))
     {
       stmtblock_t ifblock2;
@@ -3232,16 +3230,6 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
       /* Assign the value to the limit...  */
       gfc_add_modify (&ifblock, limit, arrayse.expr);
 
-      /* Remember where we are.  An offset must be added to the loop
-	 counter to obtain the required position.  */
-      if (loop.from[0])
-	tmp = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
-			       gfc_index_one_node, loop.from[0]);
-      else
-	tmp = gfc_index_one_node;
-
-      gfc_add_modify (&block, offset, tmp);
-
       tmp = fold_build2_loc (input_location, PLUS_EXPR, TREE_TYPE (pos),
 			     loop.loopvar[0], offset);
       gfc_add_modify (&ifblock, pos, tmp);

[-- Attachment #4: pr43829-58.CL --]
[-- Type: text/plain, Size: 270 bytes --]

2011-10-19  Mikael Morin  <mikael@gcc.gnu.org>

	* trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Set loop's
	temporary rank to the loop rank. Mark ss chains for multiple loop
	if necessary.  Use gfc_trans_scalarized_loop_boundary to end one loop
	and start another.

[-- Attachment #5: pr43829-58.patch --]
[-- Type: text/x-diff, Size: 2631 bytes --]

diff --git a/trans-intrinsic.c b/trans-intrinsic.c
index ee162ea..506cdf2 100644
--- a/trans-intrinsic.c
+++ b/trans-intrinsic.c
@@ -3061,6 +3061,23 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
   /* Initialize the loop.  */
   gfc_conv_ss_startstride (&loop);
+
+  /* The code generated can have more than one loop in sequence (see the
+     comment at the function header).  This doesn't work well with the
+     scalarizer, which changes arrays' offset when the scalarization loops
+     are generated (see gfc_trans_preloop_setup).  Fortunately, {min,max}loc
+     are  currently inlined in the scalar case only (for which loop is of rank
+     one).  As there is no dependency to care about in that case, there is no
+     temporary, so that we can use the scalarizer temporary code to handle
+     multiple loops.  Thus, we set temp_dim here, we call gfc_mark_ss_chain_used
+     with flag=3 later, and we use gfc_trans_scalarized_loop_boundary even later
+     to restore offset.
+     TODO: this prevents inlining of rank > 0 minmaxloc calls, so this
+     should eventually go away.  We could either create two loops properly,
+     or find another way to save/restore the array offsets between the two
+     loops (without conflicting with temporary management), or use a single
+     loop minmaxloc implementation.  See PR 31067.  */
+  loop.temp_dim = loop.dimen;
   gfc_conv_loop_setup (&loop, &expr->where);
 
   gcc_assert (loop.dimen == 1);
@@ -3098,9 +3115,9 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
 			 gfc_index_one_node, loop.from[0]);
   gfc_add_modify (&loop.pre, offset, tmp);
 
-  gfc_mark_ss_chain_used (arrayss, 1);
+  gfc_mark_ss_chain_used (arrayss, lab1 ? 3 : 1);
   if (maskss)
-    gfc_mark_ss_chain_used (maskss, 1);
+    gfc_mark_ss_chain_used (maskss, lab1 ? 3 : 1);
   /* Generate the loop body.  */
   gfc_start_scalarized_body (&loop, &body);
 
@@ -3186,7 +3203,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
   if (lab1)
     {
-      gfc_trans_scalarized_loop_end (&loop, 0, &body);
+      gfc_trans_scalarized_loop_boundary (&loop, &body);
 
       if (HONOR_NANS (DECL_MODE (limit)))
 	{
@@ -3201,7 +3218,6 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
       gfc_add_expr_to_block (&loop.code[0], build1_v (GOTO_EXPR, lab2));
       gfc_add_expr_to_block (&loop.code[0], build1_v (LABEL_EXPR, lab1));
-      gfc_start_block (&body);
 
       /* If we have a mask, only check this element if the mask is set.  */
       if (maskss)

[-- Attachment #6: pr43829-59.CL --]
[-- Type: text/plain, Size: 270 bytes --]

2011-10-19  Mikael Morin  <mikael@gcc.gnu.org>

	* trans-intrinsic.c (gfc_conv_intrinsic_minmaxval): Set loop's
	temporary rank to the loop rank. Mark ss chains for multiple loop
	if necessary.  Use gfc_trans_scalarized_loop_boundary to end one loop
	and start another.

[-- Attachment #7: pr43829-59.patch --]
[-- Type: text/x-diff, Size: 2460 bytes --]

diff --git a/trans-intrinsic.c b/trans-intrinsic.c
index 506cdf2..3cdc1e0 100644
--- a/trans-intrinsic.c
+++ b/trans-intrinsic.c
@@ -3522,6 +3522,22 @@ gfc_conv_intrinsic_minmaxval (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
   /* Initialize the loop.  */
   gfc_conv_ss_startstride (&loop);
+
+  /* The code generated can have more than one loop in sequence (see the
+     comment at the function header).  This doesn't work well with the
+     scalarizer, which changes arrays' offset when the scalarization loops
+     are generated (see gfc_trans_preloop_setup).  Fortunately, {min,max}val
+     are  currently inlined in the scalar case only.  As there is no dependency
+     to care about in that case, there is no temporary, so that we can use the
+     scalarizer temporary code to handle multiple loops.  Thus, we set temp_dim
+     here, we call gfc_mark_ss_chain_used with flag=3 later, and we use
+     gfc_trans_scalarized_loop_boundary even later to restore offset.
+     TODO: this prevents inlining of rank > 0 minmaxval calls, so this
+     should eventually go away.  We could either create two loops properly,
+     or find another way to save/restore the array offsets between the two
+     loops (without conflicting with temporary management), or use a single
+     loop minmaxval implementation.  See PR 31067.  */
+  loop.temp_dim = loop.dimen;
   gfc_conv_loop_setup (&loop, &expr->where);
 
   if (nonempty == NULL && maskss == NULL
@@ -3553,9 +3569,9 @@ gfc_conv_intrinsic_minmaxval (gfc_se * se, gfc_expr * expr, enum tree_code op)
 	}
     }
 
-  gfc_mark_ss_chain_used (arrayss, 1);
+  gfc_mark_ss_chain_used (arrayss, lab ? 3 : 1);
   if (maskss)
-    gfc_mark_ss_chain_used (maskss, 1);
+    gfc_mark_ss_chain_used (maskss, lab ? 3 : 1);
   /* Generate the loop body.  */
   gfc_start_scalarized_body (&loop, &body);
 
@@ -3665,15 +3681,13 @@ gfc_conv_intrinsic_minmaxval (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
   if (lab)
     {
-      gfc_trans_scalarized_loop_end (&loop, 0, &body);
+      gfc_trans_scalarized_loop_boundary (&loop, &body);
 
       tmp = fold_build3_loc (input_location, COND_EXPR, type, nonempty,
 			     nan_cst, huge_cst);
       gfc_add_modify (&loop.code[0], limit, tmp);
       gfc_add_expr_to_block (&loop.code[0], build1_v (LABEL_EXPR, lab));
 
-      gfc_start_block (&body);
-
       /* If we have a mask, only add this element if the mask is set.  */
       if (maskss)
 	{

  parent reply	other threads:[~2011-10-27 23:36 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-27 23:43 [Patch, fortran] [00/66] PR fortran/43829 Inline sum and product (AKA scalarization of reductions) Mikael Morin
2011-10-27 23:32 ` [Patch, fortran] [20..30/66] inline sum and product: Update core structs Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [20/66] inline sum and product: Update core structs: Rename gfc_ss_info Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [29/66] inline sum and product: Update core structs: Move useflags flag Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [25/66] inline sum and product: Update core structs: Move string_length Mikael Morin
2011-10-27 23:33   ` [Patch, fortran] [23/66] inline sum and product: Update core structs: Move type Mikael Morin
2011-10-27 23:33   ` [Patch, fortran] [26/66] inline sum and product: Update core structs: Move scalar struct Mikael Morin
2011-10-27 23:33   ` [Patch, fortran] [22/66] inline sum and product: Update core structs: Move shape Mikael Morin
2011-10-27 23:34   ` [Patch, fortran] [27/66] inline sum and product: Update core structs: Move temp struct Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [30/66] inline sum and product: Update core structs: Move where flag Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [21/66] inline sum and product: Update core structs: Move dim and dimen Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [24/66] inline sum and product: Update core structs: Move expr Mikael Morin
2011-10-27 23:43   ` [Patch, fortran] [28/66] inline sum and product: Update core structs: Move info struct Mikael Morin
2011-10-27 23:35 ` [Patch, fortran] [31..53/66] inline sum and product: Update the scalarizer Mikael Morin
2011-10-27 23:34   ` [Patch, fortran] [31/66] inline sum and product: Update the scalarizer: Split gfc_conv_loop_setup Mikael Morin
2011-10-27 23:34   ` [Patch, fortran] [32/66] inline sum and product: Update the scalarizer: clear specloop in gfc_trans_create_temp_arrays Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [45/66] inline sum and product: Update the scalarizer: Update dimension mapping inversion functions Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [33/66] inline sum and product: Update the scalarizer Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [44/66] inline sum and product: Update the scalarizer: New gfc_ss::nested_ss field Mikael Morin
2011-10-27 23:35   ` [Patch, fortran] [47..48/66] inline sum and product: Update the scalarizer: New gfc_loopinfo::nested_loop field Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [35..39/66] inline sum and product: Update the scalarizer: New gfc_ss::loop field Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [34/66] inline sum and product: Update the scalarizer: gfc_ss_info refcounting Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [46/66] inline sum and product: Update the scalarizer: Update gfc_trans_create_temp_array Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [40..43/66] inline sum and product: Update the scalarizer: New gfc_ss::parent field Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [49..51/66] inline sum and product: Update the scalarizer: New parent loop Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [52/66] inline sum and product: Update the scalarizer: New outermost_loop function Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [53/66] inline sum and product: Update the scalarizer: Update gfc_trans_preloop_setup Mikael Morin
2011-10-27 23:35 ` [Patch, fortran] [07..12/66] inline sum and product: Preliminary cleanups Mikael Morin
2011-10-27 23:31   ` [Patch, fortran] [10/66] inline sum and product: Preliminary cleanups: Use array's instead of loop's dimensions Mikael Morin
2011-10-27 23:31   ` [Patch, fortran] [07/66] inline sum and product: Preliminary cleanups: Useless coarray code removal Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [08/66] inline sum and product: Preliminary cleanups: Remove redundant condition Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [12/66] inline sum and product: Preliminary cleanups: Stop loop before end marker Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [11/66] inline sum and product: Preliminary cleanups: Skip temporary case Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [09/66] inline sum and product: Preliminary cleanups: Assertify condition Mikael Morin
2011-10-27 23:36 ` [Patch, fortran] [01..06/66] inline sum and product: Prepare gfc_trans_preloop_setup Mikael Morin
2011-10-27 23:30   ` [Patch, fortran] [06/66] " Mikael Morin
2011-10-30  9:52     ` Paul Richard Thomas
2011-10-30 21:57       ` Mikael Morin
2011-10-27 23:30   ` [Patch, fortran] [01/66] " Mikael Morin
2011-10-27 23:31   ` [Patch, fortran] [03/66] " Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [02/66] " Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [05/66] " Mikael Morin
2011-10-27 23:38   ` [Patch, fortran] [04/66] " Mikael Morin
2011-10-27 23:36 ` [Patch, fortran] [62..66/66] inline sum and product: Inline sum Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [64/66] inline sum and product: Inline sum: Change loop use Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [66/66] inline sum and product: Inline sum: The end Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [65/66] inline sum and product: Inline sum: Change se initialization Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [63/66] inline sum and product: Inline sum: Change argument handling Mikael Morin
2011-10-28  0:29   ` [Patch, fortran] [62/66] inline sum and product: Inline sum: Change conditions Mikael Morin
2011-10-28  0:02 ` [Patch, fortran] [13..19/66] inline sum and product: Interfaces changes Mikael Morin
2011-10-27 23:31   ` [Patch, fortran] [14/66] inline sum and product: Interfaces changes: gfc_trans_array_bound_check, gfc_conv_array_index_offset Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [17/66] inline sum and product: Interfaces changes: gfc_set_vector_loop_bounds Mikael Morin
2011-10-27 23:32   ` [Patch, fortran] [15/66] inline sum and product: Interfaces changes: obtain name more simply Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [13/66] inline sum and product: Interfaces changes: gfc_trans_array_constructor Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [16/66] inline sum and product: Interfaces changes: gfc_trans_create_temp_array Mikael Morin
2011-10-27 23:43   ` [Patch, fortran] [18/66] inline sum and product: Interfaces changes: get_array_ref_dim Mikael Morin
2011-10-27 23:44   ` [Patch, fortran] [19/66] inline sum and product: Interfaces changes: dim_ok Mikael Morin
2011-10-28  0:22 ` [Patch, fortran] [54..61/66] inline sum and product: Prevent regressions Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [55..56/66] inline sum and product: Prevent regressions: Fix gfc_conv_elemental_dependencies Mikael Morin
2011-10-27 23:36   ` [Patch, fortran] [60/66] inline sum and product: Update the scalarizer: Fix error markers Mikael Morin
2011-10-27 23:36   ` Mikael Morin [this message]
2011-10-27 23:43   ` [Patch, fortran] [54/66] inline sum and product: Prevent regressions: Add dependency checking Mikael Morin
2011-10-28  0:01   ` [Patch, fortran] [61/66] inline sum and product: Prevent regressions: Disable frontend optimizations Mikael Morin
2011-10-28 14:35 ` [Patch, fortran] [00/66] PR fortran/43829 Inline sum and product (AKA scalarization of reductions) Jack Howarth
2011-10-28 17:25   ` Mikael Morin
2011-10-29 16:04     ` [Patch, fortran] [00/66] PR fortran/43829 Inline sum and?product " Jack Howarth
2011-11-01 21:33 ` [Patch, fortran] [00/66] PR fortran/43829 Inline sum and product " Paul Richard Thomas
2011-11-04  3:51   ` Mikael Morin
2011-11-04  9:39     ` Richard Guenther

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=20111027233322.18581.12112@gimli.local \
    --to=mikael.morin@sfr.fr \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    /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).