Re: [PATCH] rtl-optimization/96812 - remap dependence info on RTL loop unrolling

2020-09-01 Thread Richard Biener
On Thu, 27 Aug 2020, Richard Biener wrote:

> This carries over the PR87609 fix also to RTL loop unrolling.  The
> gcc.dg/torture/pr90328.c testcase otherwise is miscompiled with
> the tree-ssa-address.c hunk (or alternatively with -fno-ivopts
> on master).  I've tried to find the correct abstraction and
> adjusted two other duplicate_insn_chain users for which I do not
> have testcases.  There may be other insn-chain copying routines
> that could be affected but hopefully most appropriately go through
> CFG hooks.
> 
> Bootstrap / regtest running on x86_64-unknown-linux-gnu.
> 
> Any comments?

None, so I'll push this later today after re-boostrap/test.

Richard.

> Thanks,
> Richard.
> 
> 2020-08-27  Richard Biener  
> 
>   PR rtl-optimization/96812
>   * tree-ssa-address.c (copy_ref_info): Also copy dependence info.
>   * cfgrtl.h (duplicate_insn_chain): Adjust prototype.
>   * cfgrtl.c (duplicate_insn_chain): Remap dependence info
>   if requested.
>   (cfg_layout_duplicate_bb): Make sure we remap dependence info.
>   * modulo-sched.c (duplicate_insns_of_cycles): Remap dependence
>   info.
>   (generate_prolog_epilog): Adjust.
>   * config/c6x/c6x.c (hwloop_optimize): Remap dependence info.
> ---
>  gcc/cfgrtl.c   | 60 ++
>  gcc/cfgrtl.h   |  3 ++-
>  gcc/config/c6x/c6x.c   |  3 ++-
>  gcc/modulo-sched.c | 10 ---
>  gcc/tree-ssa-address.c | 10 +++
>  5 files changed, 75 insertions(+), 11 deletions(-)
> 
> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index 03fa688fed6..eb5ccd42ed7 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -61,6 +61,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgloop.h"
>  #include "tree-pass.h"
>  #include "print-rtl.h"
> +#include "rtl-iter.h"
> +#include "gimplify.h"
>  
>  /* Disable warnings about missing quoting in GCC diagnostics.  */
>  #if __GNUC__ >= 10
> @@ -4199,7 +4201,8 @@ cfg_layout_can_duplicate_bb_p (const_basic_block bb)
>  }
>  
>  rtx_insn *
> -duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
> +duplicate_insn_chain (rtx_insn *from, rtx_insn *to,
> +   class loop *loop, copy_bb_data *id)
>  {
>rtx_insn *insn, *next, *copy;
>rtx_note *last;
> @@ -4228,6 +4231,51 @@ duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
> && ANY_RETURN_P (JUMP_LABEL (insn)))
>   JUMP_LABEL (copy) = JUMP_LABEL (insn);
>maybe_copy_prologue_epilogue_insn (insn, copy);
> +   /* If requested remap dependence info of cliques brought in
> +  via inlining.  */
> +   if (id)
> + {
> +   subrtx_iterator::array_type array;
> +   FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
> + if (MEM_P (*iter) && MEM_EXPR (*iter))
> +   {
> + tree op = MEM_EXPR (*iter);
> + if (TREE_CODE (op) == WITH_SIZE_EXPR)
> +   op = TREE_OPERAND (op, 0);
> + while (handled_component_p (op))
> +   op = TREE_OPERAND (op, 0);
> + if ((TREE_CODE (op) == MEM_REF
> +  || TREE_CODE (op) == TARGET_MEM_REF)
> + && MR_DEPENDENCE_CLIQUE (op) > 1
> + && (!loop
> + || (MR_DEPENDENCE_CLIQUE (op)
> + != loop->owned_clique)))
> +   {
> + if (!id->dependence_map)
> +   id->dependence_map = new hash_map +   unsigned short>;
> + bool existed;
> + unsigned short  = id->dependence_map->get_or_insert
> +  (MR_DEPENDENCE_CLIQUE (op), );
> + if (!existed)
> +   {
> + gcc_assert
> +   (MR_DEPENDENCE_CLIQUE (op) <= cfun->last_clique);
> + newc = ++cfun->last_clique;
> +   }
> + /* We cannot adjust MR_DEPENDENCE_CLIQUE in-place
> +since MEM_EXPR is shared so make a copy and
> +walk to the subtree again.  */
> + tree new_expr = unshare_expr (MEM_EXPR (*iter));
> + if (TREE_CODE (new_expr) == WITH_SIZE_EXPR)
> +   new_expr = TREE_OPERAND (new_expr, 0);
> + while (handled_component_p (new_expr))
> +   new_expr = TREE_OPERAND (new_expr, 0);
> + MR_DEPENDENCE_CLIQUE (new_expr) = newc;
> + set_mem_expr (const_cast  (*iter), new_expr);
> +   }
> +   }
> + }
> break;
>  
>   case JUMP_TABLE_DATA:
> @@ -4292,12 +4340,14 @@ duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
>  /* Create a duplicate of the basic block BB.  */
>  
>  static basic_block
> 

[PATCH] rtl-optimization/96812 - remap dependence info on RTL loop unrolling

2020-08-27 Thread Richard Biener
This carries over the PR87609 fix also to RTL loop unrolling.  The
gcc.dg/torture/pr90328.c testcase otherwise is miscompiled with
the tree-ssa-address.c hunk (or alternatively with -fno-ivopts
on master).  I've tried to find the correct abstraction and
adjusted two other duplicate_insn_chain users for which I do not
have testcases.  There may be other insn-chain copying routines
that could be affected but hopefully most appropriately go through
CFG hooks.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Any comments?

Thanks,
Richard.

2020-08-27  Richard Biener  

PR rtl-optimization/96812
* tree-ssa-address.c (copy_ref_info): Also copy dependence info.
* cfgrtl.h (duplicate_insn_chain): Adjust prototype.
* cfgrtl.c (duplicate_insn_chain): Remap dependence info
if requested.
(cfg_layout_duplicate_bb): Make sure we remap dependence info.
* modulo-sched.c (duplicate_insns_of_cycles): Remap dependence
info.
(generate_prolog_epilog): Adjust.
* config/c6x/c6x.c (hwloop_optimize): Remap dependence info.
---
 gcc/cfgrtl.c   | 60 ++
 gcc/cfgrtl.h   |  3 ++-
 gcc/config/c6x/c6x.c   |  3 ++-
 gcc/modulo-sched.c | 10 ---
 gcc/tree-ssa-address.c | 10 +++
 5 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 03fa688fed6..eb5ccd42ed7 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -61,6 +61,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-pass.h"
 #include "print-rtl.h"
+#include "rtl-iter.h"
+#include "gimplify.h"
 
 /* Disable warnings about missing quoting in GCC diagnostics.  */
 #if __GNUC__ >= 10
@@ -4199,7 +4201,8 @@ cfg_layout_can_duplicate_bb_p (const_basic_block bb)
 }
 
 rtx_insn *
-duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
+duplicate_insn_chain (rtx_insn *from, rtx_insn *to,
+ class loop *loop, copy_bb_data *id)
 {
   rtx_insn *insn, *next, *copy;
   rtx_note *last;
@@ -4228,6 +4231,51 @@ duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
  && ANY_RETURN_P (JUMP_LABEL (insn)))
JUMP_LABEL (copy) = JUMP_LABEL (insn);
   maybe_copy_prologue_epilogue_insn (insn, copy);
+ /* If requested remap dependence info of cliques brought in
+via inlining.  */
+ if (id)
+   {
+ subrtx_iterator::array_type array;
+ FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
+   if (MEM_P (*iter) && MEM_EXPR (*iter))
+ {
+   tree op = MEM_EXPR (*iter);
+   if (TREE_CODE (op) == WITH_SIZE_EXPR)
+ op = TREE_OPERAND (op, 0);
+   while (handled_component_p (op))
+ op = TREE_OPERAND (op, 0);
+   if ((TREE_CODE (op) == MEM_REF
+|| TREE_CODE (op) == TARGET_MEM_REF)
+   && MR_DEPENDENCE_CLIQUE (op) > 1
+   && (!loop
+   || (MR_DEPENDENCE_CLIQUE (op)
+   != loop->owned_clique)))
+ {
+   if (!id->dependence_map)
+ id->dependence_map = new hash_map;
+   bool existed;
+   unsigned short  = id->dependence_map->get_or_insert
+(MR_DEPENDENCE_CLIQUE (op), );
+   if (!existed)
+ {
+   gcc_assert
+ (MR_DEPENDENCE_CLIQUE (op) <= cfun->last_clique);
+   newc = ++cfun->last_clique;
+ }
+   /* We cannot adjust MR_DEPENDENCE_CLIQUE in-place
+  since MEM_EXPR is shared so make a copy and
+  walk to the subtree again.  */
+   tree new_expr = unshare_expr (MEM_EXPR (*iter));
+   if (TREE_CODE (new_expr) == WITH_SIZE_EXPR)
+ new_expr = TREE_OPERAND (new_expr, 0);
+   while (handled_component_p (new_expr))
+ new_expr = TREE_OPERAND (new_expr, 0);
+   MR_DEPENDENCE_CLIQUE (new_expr) = newc;
+   set_mem_expr (const_cast  (*iter), new_expr);
+ }
+ }
+   }
  break;
 
case JUMP_TABLE_DATA:
@@ -4292,12 +4340,14 @@ duplicate_insn_chain (rtx_insn *from, rtx_insn *to)
 /* Create a duplicate of the basic block BB.  */
 
 static basic_block
-cfg_layout_duplicate_bb (basic_block bb, copy_bb_data *)
+cfg_layout_duplicate_bb (basic_block bb, copy_bb_data *id)
 {
   rtx_insn *insn;
   basic_block new_bb;
 
-  insn = duplicate_insn_chain (BB_HEAD (bb), BB_END (bb));
+  class loop *loop = (id && current_loops) ?