rtl-ssa: Fix -fcompare-debug failure [PR100303]

This patch fixes an oversight in the handling of debug instructions
in rtl-ssa.  At the moment (and whether this is a good idea or not
remains to be seen), we maintain a linear RPO sequence of definitions
and non-debug uses.  If a register is defined more than once, we use
a degenerate phi to reestablish a previous definition where necessary.

However, debug instructions shouldn't of course affect codegen,
so we can't create a new definition just for them.  In those situations
we instead hang the debug use off the real definition (meaning that
debug uses do not follow a linear order wrt definitions).  Again,
it remains to be seen whether that's a good idea.

The problem in the PR was that we weren't taking this into account
when increasing (or potentially increasing) the live range of an
existing definition.  We'd create the phi even if it would only
be used by debug instructions.

The patch goes for the simple but inelegant approach of passing
a bool to say whether the use is a debug use or not.  I imagine
this area will need some tweaking based on experience in future.

Tested on aarch64-linux-gnu (where the test also failed).
OK to install?

Richard


gcc/
	PR rtl-optimization/100303
	* rtl-ssa/accesses.cc (function_info::make_use_available): Take a
	boolean that indicates whether the use will only be used in
	debug instructions.  Treat it in the same way that existing
	cross-EBB debug references would be handled if so.
	(function_info::make_uses_available): Likewise.
	* rtl-ssa/functions.h (function_info::make_uses_available): Update
	prototype accordingly.
	(function_info::make_uses_available): Likewise.
	* fwprop.c (try_fwprop_subst): Update call accordingly.
---
 gcc/fwprop.c                            |   3 +-
 gcc/rtl-ssa/accesses.cc                 |  15 ++--
 gcc/rtl-ssa/functions.h                 |   7 +-
 gcc/testsuite/g++.dg/torture/pr100303.C | 112 ++++++++++++++++++++++++
 4 files changed, 129 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr100303.C

On Wed, Apr 28, 2021 at 9:07 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch fixes an oversight in the handling of debug instructions
> in rtl-ssa.  At the moment (and whether this is a good idea or not
> remains to be seen), we maintain a linear RPO sequence of definitions
> and non-debug uses.  If a register is defined more than once, we use
> a degenerate phi to reestablish a previous definition where necessary.
>
> However, debug instructions shouldn't of course affect codegen,
> so we can't create a new definition just for them.  In those situations
> we instead hang the debug use off the real definition (meaning that
> debug uses do not follow a linear order wrt definitions).  Again,
> it remains to be seen whether that's a good idea.
>
> The problem in the PR was that we weren't taking this into account
> when increasing (or potentially increasing) the live range of an
> existing definition.  We'd create the phi even if it would only
> be used by debug instructions.
>
> The patch goes for the simple but inelegant approach of passing
> a bool to say whether the use is a debug use or not.  I imagine
> this area will need some tweaking based on experience in future.
>
> Tested on aarch64-linux-gnu (where the test also failed).
> OK to install?

OK.

Thanks,
Richard.

> Richard
>
>
> gcc/
>         PR rtl-optimization/100303
>         * rtl-ssa/accesses.cc (function_info::make_use_available): Take a
>         boolean that indicates whether the use will only be used in
>         debug instructions.  Treat it in the same way that existing
>         cross-EBB debug references would be handled if so.
>         (function_info::make_uses_available): Likewise.
>         * rtl-ssa/functions.h (function_info::make_uses_available): Update
>         prototype accordingly.
>         (function_info::make_uses_available): Likewise.
>         * fwprop.c (try_fwprop_subst): Update call accordingly.
> ---
>  gcc/fwprop.c                            |   3 +-
>  gcc/rtl-ssa/accesses.cc                 |  15 ++--
>  gcc/rtl-ssa/functions.h                 |   7 +-
>  gcc/testsuite/g++.dg/torture/pr100303.C | 112 ++++++++++++++++++++++++
>  4 files changed, 129 insertions(+), 8 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/torture/pr100303.C
>
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index d7203672886..73284a7ae3e 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -606,7 +606,8 @@ try_fwprop_subst (use_info *use, set_info *def,
>    if (def_insn->bb () != use_insn->bb ())
>      {
>        src_uses = crtl->ssa->make_uses_available (attempt, src_uses,
> -                                                use_insn->bb ());
> +                                                use_insn->bb (),
> +                                                use_insn->is_debug_insn ());
>        if (!src_uses.is_valid ())
>         return false;
>      }
> diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc
> index de3a29edbeb..a55cc8817a7 100644
> --- a/gcc/rtl-ssa/accesses.cc
> +++ b/gcc/rtl-ssa/accesses.cc
> @@ -1242,7 +1242,10 @@ function_info::insert_temp_clobber (obstack_watermark &watermark,
>  }
>
>  // A subroutine of make_uses_available.  Try to make USE's definition
> -// available at the head of BB.  On success:
> +// available at the head of BB.  WILL_BE_DEBUG_USE is true if the
> +// definition will be used only in debug instructions.
> +//
> +// On success:
>  //
>  // - If the use would have the same def () as USE, return USE.
>  //
> @@ -1254,7 +1257,8 @@ function_info::insert_temp_clobber (obstack_watermark &watermark,
>  //
>  // Return null on failure.
>  use_info *
> -function_info::make_use_available (use_info *use, bb_info *bb)
> +function_info::make_use_available (use_info *use, bb_info *bb,
> +                                  bool will_be_debug_use)
>  {
>    set_info *def = use->def ();
>    if (!def)
> @@ -1270,7 +1274,7 @@ function_info::make_use_available (use_info *use, bb_info *bb)
>        && single_pred (cfg_bb) == use_bb->cfg_bb ()
>        && remains_available_on_exit (def, use_bb))
>      {
> -      if (def->ebb () == bb->ebb ())
> +      if (def->ebb () == bb->ebb () || will_be_debug_use)
>         return use;
>
>        resource_info resource = use->resource ();
> @@ -1314,7 +1318,8 @@ function_info::make_use_available (use_info *use, bb_info *bb)
>  // See the comment above the declaration.
>  use_array
>  function_info::make_uses_available (obstack_watermark &watermark,
> -                                   use_array uses, bb_info *bb)
> +                                   use_array uses, bb_info *bb,
> +                                   bool will_be_debug_uses)
>  {
>    unsigned int num_uses = uses.size ();
>    if (num_uses == 0)
> @@ -1323,7 +1328,7 @@ function_info::make_uses_available (obstack_watermark &watermark,
>    auto **new_uses = XOBNEWVEC (watermark, access_info *, num_uses);
>    for (unsigned int i = 0; i < num_uses; ++i)
>      {
> -      use_info *use = make_use_available (uses[i], bb);
> +      use_info *use = make_use_available (uses[i], bb, will_be_debug_uses);
>        if (!use)
>         return use_array (access_array::invalid ());
>        new_uses[i] = use;
> diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h
> index 58755d74cc9..cf68c94647a 100644
> --- a/gcc/rtl-ssa/functions.h
> +++ b/gcc/rtl-ssa/functions.h
> @@ -126,8 +126,11 @@ public:
>    // If the operation fails, return an invalid use_array.
>    //
>    // WATERMARK is a watermark returned by new_change_attempt ().
> +  // WILL_BE_DEBUG_USES is true if the returned use_array will be
> +  // used only for debug instructions.
>    use_array make_uses_available (obstack_watermark &watermark,
> -                                use_array uses, bb_info *bb);
> +                                use_array uses, bb_info *bb,
> +                                bool will_be_debug_uses);
>
>    // If CHANGE doesn't already clobber REGNO, try to add such a clobber,
>    // limiting the movement range in order to make the clobber valid.
> @@ -196,7 +199,7 @@ private:
>    def_node *need_def_node (def_info *);
>    def_splay_tree need_def_splay_tree (def_info *);
>
> -  use_info *make_use_available (use_info *, bb_info *);
> +  use_info *make_use_available (use_info *, bb_info *, bool);
>    def_array insert_temp_clobber (obstack_watermark &, insn_info *,
>                                  unsigned int, def_array);
>
> diff --git a/gcc/testsuite/g++.dg/torture/pr100303.C b/gcc/testsuite/g++.dg/torture/pr100303.C
> new file mode 100644
> index 00000000000..5af9412df40
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr100303.C
> @@ -0,0 +1,112 @@
> +/* { dg-additional-options "-w -fcompare-debug -fno-dce -ftracer" } */
> +
> +template < typename _T1 > struct pair
> +{
> +  _T1 first;
> +  int second;
> +};
> +struct __aligned_membuf
> +{
> +  void _M_ptr ();
> +};
> +struct _Rb_tree_node_base
> +{
> +  typedef _Rb_tree_node_base *_Base_ptr;
> +};
> +struct _Rb_tree_node:_Rb_tree_node_base
> +{
> +  __aligned_membuf _M_storage;
> +  void _M_valptr ()
> +  {
> +    _M_storage._M_ptr ();
> +  }
> +};
> +struct _Rb_tree_iterator
> +{
> +  typedef _Rb_tree_node_base::_Base_ptr _Base_ptr;
> +    _Rb_tree_iterator (_Base_ptr __x):_M_node (__x)
> +  {
> +  }
> +  void operator* ()
> +  {
> +    static_cast < _Rb_tree_node * >(_M_node)->_M_valptr ();
> +  }
> +  friend bool operator== (_Rb_tree_iterator __x, _Rb_tree_iterator)
> +  {
> +    return __x._M_node;
> +  }
> +  _Base_ptr _M_node;
> +};
> +
> +template < typename, typename, typename, typename, typename =
> +  int >class _Rb_tree
> +{
> +  typedef _Rb_tree_node_base *_Base_ptr;
> +public:
> +    pair < _Base_ptr > _M_get_insert_hint_unique_pos (int);
> +  void _M_insert_node (_Base_ptr, int);
> +    template < typename ... _Args >
> +    _Rb_tree_iterator _M_emplace_hint_unique (_Args && ...);
> +  _Rb_tree_iterator lower_bound ()
> +  {
> +    _Rb_tree_node_base __trans_tmp_2;
> +      return &__trans_tmp_2;
> +  }
> +};
> +template < typename _Key, typename _Val, typename _KeyOfValue,
> +  typename _Compare,
> +  typename _Alloc > template < typename ... _Args >
> +  _Rb_tree_iterator _Rb_tree < _Key, _Val, _KeyOfValue, _Compare,
> +  _Alloc >::_M_emplace_hint_unique (_Args && ...)
> +{
> +  int __z;
> +  try
> +  {
> +    auto __res = _M_get_insert_hint_unique_pos (0);
> +    _Rb_tree_node_base *__res_1;
> +    if (__res_1)
> +      _M_insert_node (__res.first, __z);
> +    return __res.first;
> +  }
> +  catch ( ...)
> +  {
> +  }
> +}
> +
> +class map
> +{
> +  _Rb_tree < int, int, int, int >_M_t;
> +public:
> +    _Rb_tree_iterator end ();
> +  void operator[] (int)
> +  {
> +    _Rb_tree_iterator __i = lower_bound ();
> +    if (__i == end ())
> +      __i = _M_t._M_emplace_hint_unique (__i);
> +    *__i;
> +  }
> +  _Rb_tree_iterator lower_bound ()
> +  {
> +    return _M_t.lower_bound ();
> +  }
> +};
> +
> +class FlowStat
> +{
> +public:
> +  int FlowStat_flow;
> +    FlowStat ()
> +  {
> +    shares[FlowStat_flow];
> +  }
> +  map shares;
> +};
> +
> +class LinkGraphJob
> +{
> +  ~LinkGraphJob ();
> +};
> +LinkGraphJob::~LinkGraphJob ()
> +{
> +  FlowStat ();
> +}