[PATCH GCC][5/6]Record initialization statements and only insert it for valid chains

[PATCH GCC][5/6]Record initialization statements and only insert it for valid chains

[Bin Cheng]

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

Hi,
This patch caches initialization statements and only inserts it for valid chains.
Looks like current code even inserts such stmts for invalid chains which will be
deleted as dead code afterwards.

Bootstrap and test on x86_64 and AArch64, is it OK?

Thanks,
bin
2017-05-10  Bin Cheng  <bin.cheng@arm.com>

	* tree-predcom.c (struct chain): New field init_seq.
	(prepare_initializers_chain): Record intialization stmts in above
	field.  Discard it if chain is invalid.
	(insert_init_seqs): New function.
	(tree_predictive_commoning_loop): Call insert_init_seqs.

[-- Attachment #2: 0005-chain-init-seq-20170102.txt --]
[-- Type: text/plain, Size: 2230 bytes --]

From db10e77e52ad44416edc08f9789faabef911a9d8 Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Mon, 24 Apr 2017 14:48:12 +0100
Subject: [PATCH 5/6] chain-init-seq-20170102.txt

---
 gcc/tree-predcom.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c
index 0b7811a..f73d869 100644
--- a/gcc/tree-predcom.c
+++ b/gcc/tree-predcom.c
@@ -296,6 +296,9 @@ typedef struct chain
   /* Initializers for the variables.  */
   vec<tree> inits;
 
+  /* gimple stmts intializing the initial variables of the chain.  */
+  gimple_seq init_seq;
+
   /* True if there is a use of a variable with the maximal distance
      that comes after the root in the loop.  */
   unsigned has_max_use_after : 1;
@@ -2454,12 +2457,14 @@ prepare_initializers_chain (struct loop *loop, chain_p chain)
       init = ref_at_iteration (dr, (int) i - n, &stmts);
       if (!chain->all_always_accessed && tree_could_trap_p (init))
 	{
+	  if (chain->init_seq)
+	    gimple_seq_discard (chain->init_seq);
 	  gimple_seq_discard (stmts);
 	  return false;
 	}
 
       if (stmts)
-	gsi_insert_seq_on_edge_immediate (entry, stmts);
+	gimple_seq_add_seq (&chain->init_seq, stmts);
 
       chain->inits[i] = init;
     }
@@ -2489,6 +2494,22 @@ prepare_initializers (struct loop *loop, vec<chain_p> chains)
     }
 }
 
+/* Insert all initializing gimple stmts into loop's entry edge.  */
+
+static void
+insert_init_seqs (struct loop *loop, vec<chain_p> chains)
+{
+  unsigned i;
+  edge entry = loop_preheader_edge (loop);
+
+  for (i = 0; i < chains.length (); ++i)
+    if (chains[i]->init_seq)
+      {
+	gsi_insert_seq_on_edge_immediate (entry, chains[i]->init_seq);
+	chains[i]->init_seq = NULL;
+      }
+}
+
 /* Performs predictive commoning for LOOP.  Returns true if LOOP was
    unrolled.  */
 
@@ -2571,6 +2592,8 @@ tree_predictive_commoning_loop (struct loop *loop)
   /* Try to combine the chains that are always worked with together.  */
   try_combine_chains (&chains);
 
+  insert_init_seqs (loop, chains);
+
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Before commoning:\n\n");
-- 
1.9.1

Re: [PATCH GCC][5/6]Record initialization statements and only insert it for valid chains

[Bin.Cheng]

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

On Fri, May 12, 2017 at 12:28 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> This patch caches initialization statements and only inserts it for valid chains.
> Looks like current code even inserts such stmts for invalid chains which will be
> deleted as dead code afterwards.
>
> Bootstrap and test on x86_64 and AArch64, is it OK?

Ping this one because it's a prerequisite patch for following predcom
enhancement.
Also I updated the patch a little bit.

Bootstrap and test along with following patches of predcom on x86_64
and AArch64.  Is it OK?

Thanks,
bin
2017-06-26  Bin Cheng  <bin.cheng@arm.com>

    * tree-predcom.c (struct chain): New field init_seq.
    (release_chain): Release init_seq.
    (prepare_initializers_chain): Record intialization stmts in above
    field.  Discard it if chain is invalid.
    (insert_init_seqs): New function.
    (tree_predictive_commoning_loop): Call insert_init_seqs.

[-- Attachment #2: 0002-chain-init-seq-20170620.txt.patch --]
[-- Type: text/x-patch, Size: 2200 bytes --]

From df8db56c584864ab6c8e6c2b7dcab2d57daf830a Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Mon, 26 Jun 2017 10:33:18 +0100
Subject: [PATCH 2/6] chain-init-seq-20170620.txt

---
 gcc/tree-predcom.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c
index 4547b6d..260caaf 100644
--- a/gcc/tree-predcom.c
+++ b/gcc/tree-predcom.c
@@ -294,6 +294,9 @@ typedef struct chain
   /* Initializers for the variables.  */
   vec<tree> inits;
 
+  /* gimple stmts intializing the initial variables of the chain.  */
+  gimple_seq init_seq;
+
   /* True if there is a use of a variable with the maximal distance
      that comes after the root in the loop.  */
   unsigned has_max_use_after : 1;
@@ -511,6 +514,8 @@ release_chain (chain_p chain)
   chain->refs.release ();
   chain->vars.release ();
   chain->inits.release ();
+  if (chain->init_seq)
+    gimple_seq_discard (chain->init_seq);
 
   free (chain);
 }
@@ -2457,7 +2462,7 @@ prepare_initializers_chain (struct loop *loop, chain_p chain)
 	}
 
       if (stmts)
-	gsi_insert_seq_on_edge_immediate (entry, stmts);
+	gimple_seq_add_seq (&chain->init_seq, stmts);
 
       chain->inits[i] = init;
     }
@@ -2487,6 +2492,22 @@ prepare_initializers (struct loop *loop, vec<chain_p> chains)
     }
 }
 
+/* Insert all initializing gimple stmts into loop's entry edge.  */
+
+static void
+insert_init_seqs (struct loop *loop, vec<chain_p> chains)
+{
+  unsigned i;
+  edge entry = loop_preheader_edge (loop);
+
+  for (i = 0; i < chains.length (); ++i)
+    if (chains[i]->init_seq)
+      {
+	gsi_insert_seq_on_edge_immediate (entry, chains[i]->init_seq);
+	chains[i]->init_seq = NULL;
+      }
+}
+
 /* Performs predictive commoning for LOOP.  Returns true if LOOP was
    unrolled.  */
 
@@ -2568,6 +2589,8 @@ tree_predictive_commoning_loop (struct loop *loop)
   /* Try to combine the chains that are always worked with together.  */
   try_combine_chains (&chains);
 
+  insert_init_seqs (loop, chains);
+
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Before commoning:\n\n");
-- 
1.9.1

Re: [PATCH GCC][5/6]Record initialization statements and only insert it for valid chains

[Richard Biener]

On Mon, Jun 26, 2017 at 11:47 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Fri, May 12, 2017 at 12:28 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> Hi,
>> This patch caches initialization statements and only inserts it for valid chains.
>> Looks like current code even inserts such stmts for invalid chains which will be
>> deleted as dead code afterwards.
>>
>> Bootstrap and test on x86_64 and AArch64, is it OK?
>
> Ping this one because it's a prerequisite patch for following predcom
> enhancement.
> Also I updated the patch a little bit.
>
> Bootstrap and test along with following patches of predcom on x86_64
> and AArch64.  Is it OK?

       if (stmts)
-       gsi_insert_seq_on_edge_immediate (entry, stmts);
+       gimple_seq_add_seq (&chain->init_seq, stmts);

use gimple_seq_add_seq_without_update.

Ok with that change.

Thanks,
Richard.

> Thanks,
> bin
> 2017-06-26  Bin Cheng  <bin.cheng@arm.com>
>
>     * tree-predcom.c (struct chain): New field init_seq.
>     (release_chain): Release init_seq.
>     (prepare_initializers_chain): Record intialization stmts in above
>     field.  Discard it if chain is invalid.
>     (insert_init_seqs): New function.
>     (tree_predictive_commoning_loop): Call insert_init_seqs.

Re: [PATCH GCC][5/6]Record initialization statements and only insert it for valid chains

[Bin.Cheng]

On Mon, Jun 26, 2017 at 10:57 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Jun 26, 2017 at 11:47 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Fri, May 12, 2017 at 12:28 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>>> Hi,
>>> This patch caches initialization statements and only inserts it for valid chains.
>>> Looks like current code even inserts such stmts for invalid chains which will be
>>> deleted as dead code afterwards.
>>>
>>> Bootstrap and test on x86_64 and AArch64, is it OK?
>>
>> Ping this one because it's a prerequisite patch for following predcom
>> enhancement.
>> Also I updated the patch a little bit.
>>
>> Bootstrap and test along with following patches of predcom on x86_64
>> and AArch64.  Is it OK?
>
>        if (stmts)
> -       gsi_insert_seq_on_edge_immediate (entry, stmts);
> +       gimple_seq_add_seq (&chain->init_seq, stmts);
>
> use gimple_seq_add_seq_without_update.
Patch revised as suggested and retested.  Applied as 250666.

Thanks,
bin
>
> Ok with that change.
>
> Thanks,
> Richard.
>
>> Thanks,
>> bin
>> 2017-06-26  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * tree-predcom.c (struct chain): New field init_seq.
>>     (release_chain): Release init_seq.
>>     (prepare_initializers_chain): Record intialization stmts in above
>>     field.  Discard it if chain is invalid.
>>     (insert_init_seqs): New function.
>>     (tree_predictive_commoning_loop): Call insert_init_seqs.