diff mbox series

regrename: Fix -fcompare-debug issue in find_rename_reg [PR103756]

Message ID 20211229101901.GU2646553@tucnak
State New
Headers show
Series regrename: Fix -fcompare-debug issue in find_rename_reg [PR103756] | expand

Commit Message

Jakub Jelinek Dec. 29, 2021, 10:19 a.m. UTC 
Hi!

The r12-5978 change caused a -fcompare-debug issue, because without
-g a chain might start with a noop move, but with -g there could be
one or more DEBUG_INSNs in the chain before the noop move and so
regrename could make different decisions between -g and -g0.

Note, I must say I don't really understand the original change much,
if we want to make sure the noop moves are removed, couldn't regrename
during building of those du chains simply remove the noop moves instead?

Anyway, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-12-28  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/103756
	* regrename.c (find_rename_reg): Test noop_move_p on the first
	non-debug insn in the chain rather than on the first insn.

	* g++.dg/opt/pr103756.C: New test.


	Jakub

Comments

Jeff Law Dec. 30, 2021, 4:33 p.m. UTC | #1 
On 12/29/2021 3:19 AM, Jakub Jelinek wrote:
> Hi!
>
> The r12-5978 change caused a -fcompare-debug issue, because without
> -g a chain might start with a noop move, but with -g there could be
> one or more DEBUG_INSNs in the chain before the noop move and so
> regrename could make different decisions between -g and -g0.
>
> Note, I must say I don't really understand the original change much,
> if we want to make sure the noop moves are removed, couldn't regrename
> during building of those du chains simply remove the noop moves instead?
>
> Anyway, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2021-12-28  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/103756
> 	* regrename.c (find_rename_reg): Test noop_move_p on the first
> 	non-debug insn in the chain rather than on the first insn.
>
> 	* g++.dg/opt/pr103756.C: New test.
Sadly I wasn't ever able to trigger a nop-move in this code to 
investigate how the nop-move  more thoroughly.  IIRC Jojo indicated it 
occurred with some local changes he was playing with.

The patch is fine, of course.

jeff
diff mbox series

Patch

--- gcc/regrename.c.jj	2021-12-15 10:23:48.214517305 +0100
+++ gcc/regrename.c	2021-12-28 18:53:50.698355505 +0100
@@ -394,10 +394,15 @@  find_rename_reg (du_head_p this_head, en
 			  this_head, *unavailable))
     return this_head->tied_chain->regno;
 
-  /* If this insn is a noop move, then do not rename in this chain as doing so
-     would inhibit removal of the noop move.  */
-  if (noop_move_p (this_head->first->insn))
-    return best_new_reg;
+  /* If the first non-debug insn is a noop move, then do not rename in this
+     chain as doing so would inhibit removal of the noop move.  */
+  for (struct du_chain *tmp = this_head->first; tmp; tmp = tmp->next_use)
+    if (DEBUG_INSN_P (tmp->insn))
+      continue;
+    else if (noop_move_p (tmp->insn))
+      return best_new_reg;
+    else
+      break;
 
   /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
      over registers that belong to PREFERRED_CLASS and try to find the
--- gcc/testsuite/g++.dg/opt/pr103756.C.jj	2021-12-28 18:52:35.781397335 +0100
+++ gcc/testsuite/g++.dg/opt/pr103756.C	2021-12-28 18:51:07.285627994 +0100
@@ -0,0 +1,57 @@ 
+// PR rtl-optimization/103756
+// { dg-do compile }
+// { dg-options "-std=c++17 -O -fcompare-debug -fconserve-stack -frename-registers -fno-tree-ch -fira-algorithm=priority" }
+
+char __copy_move_b___trans_tmp_9;
+template <typename> struct __iterator_traits;
+template <typename _Tp> struct __iterator_traits<_Tp *> {
+  typedef _Tp &reference;
+};
+template <typename _Iterator> struct reverse_iterator {
+  _Iterator current;
+  reverse_iterator();
+  reverse_iterator(reverse_iterator &__x) : current(__x.current) {}
+  _Iterator base() { return current; }
+  typename __iterator_traits<_Iterator>::reference operator*() {
+    return *current;
+  }
+  reverse_iterator operator--() {
+    ++current;
+    return *this;
+  }
+};
+template <typename _IteratorL, typename _IteratorR>
+auto operator-(_IteratorL __x, _IteratorR __y) {
+  return __y - __x.base();
+}
+struct __copy_move_backward {
+  template <typename _BI1, typename _BI2>
+  static _BI2 __copy_move_b(_BI1 __first, _BI1 __last, _BI2 __result) {
+    typename __n = __last - __first;
+    for (; __n > 0; --__n) {
+      reverse_iterator __trans_tmp_8 = --__result;
+      *__trans_tmp_8 = __copy_move_b___trans_tmp_9;
+    }
+    return __result;
+  }
+};
+template <int, typename _BI1, typename _BI2>
+inline _BI2 __copy_move_backward_a2(_BI1 __first, _BI1 __last, _BI2 __result) {
+  return __copy_move_backward::__copy_move_b(__first, __last, __result);
+}
+template <int _IsMove, typename _BI1, typename _BI2>
+_BI2 __copy_move_backward_a1(_BI1 __last, _BI2 __result) {
+  _BI1 __first;
+  return __copy_move_backward_a2<_IsMove>(__first, __last, __result);
+}
+template <int _IsMove, typename _II, typename _OI>
+void __copy_move_backward_a(_II, _OI __result) {
+  reverse_iterator<unsigned char *> __trans_tmp_7 =
+      __copy_move_backward_a1<_IsMove>(__trans_tmp_7, __result);
+}
+template <typename _BI1, typename _BI2>
+void move_backward(_BI1 __first, _BI2 __result) {
+  __copy_move_backward_a<true>(__first, __result);
+}
+reverse_iterator<unsigned char *> __rotate___first;
+void __rotate() { move_backward(__rotate___first, __rotate___first); }