public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Hans-Peter Nilsson <hp@axis.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: <gcc-patches@gcc.gnu.org>
Subject: [REVERTED] testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement
Date: Thu, 11 Apr 2024 01:16:32 +0200	[thread overview]
Message-ID: <20240410231632.26E9820442@pchp3.se.axis.com> (raw)
In-Reply-To: <20240409201810.GM19790@gate.crashing.org> (message from Segher Boessenkool on Tue, 9 Apr 2024 15:18:10 -0500)

> Date: Tue, 9 Apr 2024 15:18:10 -0500
> From: Segher Boessenkool <segher@kernel.crashing.org>

> All (target-specific) new testsuite failures are just like that: bad
> testcases!

With a touch of bad assumptions by port-specific code, no
doubt.  Maybe also rtx costs including my pet peeve, the
default implementation of insn_costs (the one that doesn't
look at the destination of setters and which when you try
fixing it, pulls you down a rabbit-hole of cost-related
regressions that even Bernd S. backed away from).

> So no, no reversion.

(...)

> > That's the only test that's improved to the point of
> > affecting test-patterns.  E.g. pr93372-5.c (which references
> > pr93372-2.c) is also improved, though it retains a redundant
> > compare insn.  (PR 93372 was about regressions from the cc0
> > representation; not further improvement like here, thus it's
> > not tagged.  Though, I did not double-check whether this
> > actually *was* a regression from cc0.)
> 
> Interesting that this improved tests for you.  Huh.  Do you have an
> explanation how this happened?

Just a hunch: less combine churn (more straightforward code)
made cmpelim's job easier, same thing you wrote in order
words:

>  I suspect that as uaual it is just a
> side effect of random factors: combine is opportunistic, always does the
> first change it thinks good, not considering what this then does for
> other possible combinations; it is greedy.  It would be nice to see
> written out what happens in this example though :-)

Yes it would, but I have other things on my plate.  Besides,
it's your patch, can't rob you of the fun.

I committed the revert below, but hope to re-apply
(re-revert) it in stage 1, when as per Richard B's message
the combine improvement will reappear.

brgds, H-P

-- >8 --
From: Hans-Peter Nilsson <hp@axis.com>
Date: Wed, 10 Apr 2024 17:24:10 +0200
Subject: [PATCH] Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass
 from combine improvement"

This reverts commit 4c8b3600c4856f7915281ae3ff4d97271c83a540.
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 2ef6471a990b..912069c018d5 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,20 +1,19 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
-/* { dg-final { scan-assembler-not "\tnot" } } */
-/* { dg-final { scan-assembler-not "\tlsr" } } */
-/* We should get just one move, storing the result into *d.  */
-/* { dg-final { scan-assembler-times "\tmove" 1 } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* We used to get a cmp.d with the original operands here. */
+  /* Whoops!  We get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
-     (a.k.a "spl") re-using flags from the subtraction. */
+  /* Whoops!  While we don't get a test.d for the result here for cc0,
+     we get a sequence of insns: a move, a "not" and a shift of the
+     subtraction-result, where a simple "spl" would have done. */
   return c >= 0;
 }
-- 
2.30.2



brgds, H-P

  reply	other threads:[~2024-04-10 23:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05  2:06 [COMMITTED] " Hans-Peter Nilsson
2024-04-09 20:18 ` Segher Boessenkool
2024-04-10 23:16   ` Hans-Peter Nilsson [this message]
2024-05-08  2:26     ` [COMMITTED] Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement"" " Hans-Peter Nilsson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240410231632.26E9820442@pchp3.se.axis.com \
    --to=hp@axis.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=segher@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).