Re: [PATCH] rtl-optimization/96812 - remap dependence info on RTL loop unrolling
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
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) ?