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

[Richard Biener <richard.guenther@gmail.com>]

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 ();
> +}