* [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544]
@ 2022-02-16 8:53 Jakub Jelinek
2022-02-16 10:44 ` Segher Boessenkool
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-02-16 8:53 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
Hi!
On the following testcase on aarch64-linux, we behave differently
with -g and -g0.
The problem is that on:
(insn 10011 10010 10012 2 (set (reg:CC 66 cc)
(compare:CC (reg:DI 105)
(const_int 0 [0]))) "pr104544.c":18:3 407 {cmpdi}
(expr_list:REG_DEAD (reg:DI 105)
(nil)))
(insn 10012 10011 10013 2 (set (reg:SI 109)
(eq:SI (reg:CC 66 cc)
(const_int 0 [0]))) "pr104544.c":18:3 444 {aarch64_cstoresi}
(expr_list:REG_DEAD (reg:CC 66 cc)
(nil)))
(insn 10013 10012 10016 2 (set (reg:DI 110)
(zero_extend:DI (reg:SI 109))) "pr104544.c":18:3 111 {*zero_extendsidi2_aarch64}
(expr_list:REG_DEAD (reg:SI 109)
(nil)))
(insn 10016 10013 10017 2 (parallel [
(set (reg:CC 66 cc)
(compare:CC (const_int 0 [0])
(reg:DI 110)))
(set (reg:DI 111)
(neg:DI (reg:DI 110)))
]) "pr104544.c":18:3 281 {negdi_carryout}
(expr_list:REG_DEAD (reg:DI 110)
(nil)))
...
(debug_insn 6 5 7 2 (var_location:SI y (debug_expr:SI D#5)) "pr104544.c":18:3 -1
(nil))
(debug_insn 7 6 10033 2 (debug_marker) "pr104544.c":11:3 -1
(nil))
(insn 10033 7 10034 2 (set (reg:DI 117 [ _14 ])
(ior:DI (reg:DI 111)
(reg:DI 112))) "pr104544.c":11:6 496 {iordi3}
(expr_list:REG_DEAD (reg:DI 112)
(expr_list:REG_DEAD (reg:DI 111)
(nil))))
we successfully split 3 insns into two:
Trying 10011, 10013 -> 10016:
10011: cc:CC=cmp(r105:DI,0)
REG_DEAD r105:DI
10013: r110:DI=cc:CC==0
REG_DEAD cc:CC
10016: {cc:CC=cmp(0,r110:DI);r111:DI=-r110:DI;}
REG_DEAD r110:DI
Failed to match this instruction:
(parallel [
(set (reg:CC 66 cc)
(compare:CC (reg:DI 105)
(const_int 0 [0])))
(set (reg:DI 111)
(neg:DI (eq:DI (reg:DI 105)
(const_int 0 [0]))))
])
Failed to match this instruction:
(parallel [
(set (reg:CC 66 cc)
(compare:CC (reg:DI 105)
(const_int 0 [0])))
(set (reg:DI 111)
(neg:DI (eq:DI (reg:DI 105)
(const_int 0 [0]))))
])
Successfully matched this instruction:
(set (reg:DI 111)
(neg:DI (eq:DI (reg:DI 105)
(const_int 0 [0]))))
Successfully matched this instruction:
(set (reg:CC 66 cc)
(compare:CC (reg:DI 105)
(const_int 0 [0])))
Successfully matched this instruction:
(set (reg:DI 112)
(neg:DI (eq:DI (reg:CC 66 cc)
(const_int 0 [0]))))
allowing combination of insns 10011, 10013 and 10016
original costs 4 + 4 + 4 = 16
replacement costs 4 + 4 = 12
deferring deletion of insn with uid = 10011.
but the code that searches forward for insns to update their log
links (before the change there is a link from insn 10033 to insn 10016
for pseudo 111) only finds insn 10033 and updates the log link if
-g isn't enabled, otherwise it stops earlier because there are debug insns
in between. So, with -g LOG_LINKS of 10033 isn't updated, points eventually
to NOTE_INSN_DELETED and so we do not attempt to combine 10033 with other
insns, while with -g0 we do.
The following patch fixes that by instead ignoring debug insns during the
searching. We can still check BLOCK_FOR_INSN (insn) on those, because
if we notice DEBUG_INSN in a following basic block, necessarily there won't
be any further normal insns in the current block after it.
Bootstrapped/regtested on x86_64-linux and i686-linux, bootstrapped
on aarch64-linux, regtest on aarch64-linux still pending, ok for trunk
if it succeeds?
2022-02-16 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/104544
* combine.cc (try_combine): When looking for insn whose links
should be updated from i3 to i2, don't stop on debug insns, instead
skip over them.
* gcc.dg/pr104544.c: New test.
--- gcc/combine.cc.jj 2022-02-11 13:51:56.294928090 +0100
+++ gcc/combine.cc 2022-02-15 14:15:41.663012950 +0100
@@ -4223,10 +4223,12 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
for (rtx_insn *insn = NEXT_INSN (i3);
!done
&& insn
- && NONDEBUG_INSN_P (insn)
+ && INSN_P (insn)
&& BLOCK_FOR_INSN (insn) == this_basic_block;
insn = NEXT_INSN (insn))
{
+ if (DEBUG_INSN_P (insn))
+ continue;
struct insn_link *link;
FOR_EACH_LOG_LINK (link, insn)
if (link->insn == i3 && link->regno == regno)
--- gcc/testsuite/gcc.dg/pr104544.c.jj 2022-02-15 14:17:50.154221461 +0100
+++ gcc/testsuite/gcc.dg/pr104544.c 2022-02-15 14:17:34.441440536 +0100
@@ -0,0 +1,19 @@
+/* PR rtl-optimization/104544 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int m, n;
+__int128 q;
+
+void
+bar (unsigned __int128 x, int y)
+{
+ if (x)
+ q += y;
+}
+
+void
+foo (void)
+{
+ bar (!!q - 1, (m += m ? m : 1) < n);
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544]
2022-02-16 8:53 [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544] Jakub Jelinek
@ 2022-02-16 10:44 ` Segher Boessenkool
2022-02-16 10:55 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Segher Boessenkool @ 2022-02-16 10:44 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
Hi!
On Wed, Feb 16, 2022 at 09:53:34AM +0100, Jakub Jelinek wrote:
> On the following testcase on aarch64-linux, we behave differently
> with -g and -g0.
[ huge snip ]
> The following patch fixes that by instead ignoring debug insns during the
> searching. We can still check BLOCK_FOR_INSN (insn) on those, because
> if we notice DEBUG_INSN in a following basic block, necessarily there won't
> be any further normal insns in the current block after it.
> --- gcc/combine.cc.jj 2022-02-11 13:51:56.294928090 +0100
> +++ gcc/combine.cc 2022-02-15 14:15:41.663012950 +0100
> @@ -4223,10 +4223,12 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> for (rtx_insn *insn = NEXT_INSN (i3);
> !done
> && insn
> - && NONDEBUG_INSN_P (insn)
> + && INSN_P (insn)
> && BLOCK_FOR_INSN (insn) == this_basic_block;
> insn = NEXT_INSN (insn))
> {
> + if (DEBUG_INSN_P (insn))
> + continue;
> struct insn_link *link;
> FOR_EACH_LOG_LINK (link, insn)
> if (link->insn == i3 && link->regno == regno)
About half of the similar loops in combine.c are still broken this way,
from a quick sampling :-(
Okay for trunk and all backports you may want. Thanks!
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544]
2022-02-16 10:44 ` Segher Boessenkool
@ 2022-02-16 10:55 ` Jakub Jelinek
2022-02-16 15:11 ` Segher Boessenkool
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-02-16 10:55 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On Wed, Feb 16, 2022 at 04:44:58AM -0600, Segher Boessenkool wrote:
> > --- gcc/combine.cc.jj 2022-02-11 13:51:56.294928090 +0100
> > +++ gcc/combine.cc 2022-02-15 14:15:41.663012950 +0100
> > @@ -4223,10 +4223,12 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> > for (rtx_insn *insn = NEXT_INSN (i3);
> > !done
> > && insn
> > - && NONDEBUG_INSN_P (insn)
> > + && INSN_P (insn)
> > && BLOCK_FOR_INSN (insn) == this_basic_block;
> > insn = NEXT_INSN (insn))
> > {
> > + if (DEBUG_INSN_P (insn))
> > + continue;
> > struct insn_link *link;
> > FOR_EACH_LOG_LINK (link, insn)
> > if (link->insn == i3 && link->regno == regno)
>
> About half of the similar loops in combine.c are still broken this way,
> from a quick sampling :-(
Looking for just NONDEBUG_INSN_P, I don't see any other than this.
> Okay for trunk and all backports you may want. Thanks!
Thanks.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544]
2022-02-16 10:55 ` Jakub Jelinek
@ 2022-02-16 15:11 ` Segher Boessenkool
0 siblings, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2022-02-16 15:11 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Wed, Feb 16, 2022 at 11:55:23AM +0100, Jakub Jelinek wrote:
> On Wed, Feb 16, 2022 at 04:44:58AM -0600, Segher Boessenkool wrote:
> > About half of the similar loops in combine.c are still broken this way,
> > from a quick sampling :-(
>
> Looking for just NONDEBUG_INSN_P, I don't see any other than this.
Ah yes, I was confused by !NONDEBUG_INSN. Too many inversions make my
head spin (NONDEBUG_INSN really means RTX_INSN && !DEBUG_INSN).
So everything looks fine here now. Thanks for double checking!
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-16 15:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-16 8:53 [PATCH] combine: Fix up -fcompare-debug issue in the combiner [PR104544] Jakub Jelinek
2022-02-16 10:44 ` Segher Boessenkool
2022-02-16 10:55 ` Jakub Jelinek
2022-02-16 15:11 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).