From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id 9813E3857033 for ; Thu, 29 Apr 2021 10:23:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9813E3857033 Received: by mail-ej1-x629.google.com with SMTP id gx5so6978494ejb.11 for ; Thu, 29 Apr 2021 03:23:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=IrqXKClv5gcb7KFY+VqFF1xXFfnxQTEt9sJYzGhGbbw=; b=REJx8HUkd1V0PhszPiOwFLxJmvCU1WH66Wjwt1DUSOJop8/sJRmhQarfFhTawN8Kwz tQSZAgB/BtB4ZWblBO93o7p4LJgCjEkORRr8EcFwxdKkqcVgyubfFmrpXqd0bIXi1UNB EGvf3LkuLJzxSXgrmdxDN1J3zVrdP1usuaUeX9fw7rFx7S/xQPTcOlBACQdQhj+UIkmz 3RYP+7zMeJiNXRN6xbQbo7lPB3A6GeyKIKOVHLf6D1Zn541T3DiYq5hDODxQlL0ZGZVD BU35Qo7fdejWUR3ZMWGCcIDKvSXRCD7T0K6Qc3KK4MK+4NBnq96r6bmiPaoukDrbf+Pc bmmQ== X-Gm-Message-State: AOAM533JUx5V+fZfsQKVVQF/ns9Xy3mTO42fRVMlvYu3dlvdnv90H76i cfXIZY7b0kvjx0iAC/VXvgO8+h8lpeLB6SEK2uQ5FhIYNNY= X-Google-Smtp-Source: ABdhPJyXogmXgjVet5VoABBQKumF4xELh2slFMh4R6ysvTtsna91rvzkYpzPGaoyP1qzQK74oFJUK3EbbgMNoOthfYU= X-Received: by 2002:a17:906:36d1:: with SMTP id b17mr35290624ejc.235.1619691791671; Thu, 29 Apr 2021 03:23:11 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 29 Apr 2021 12:23:00 +0200 Message-ID: Subject: Re: [PATCH] rtl-ssa: Fix -fcompare-debug failure [PR100303] To: Richard Sandiford , GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Apr 2021 10:23:14 -0000 On Wed, Apr 28, 2021 at 9:07 PM Richard Sandiford via Gcc-patches 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 (); > +}