public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0)))
@ 2015-05-11 13:13 Segher Boessenkool
  2015-05-11 17:23 ` Jeff Law
  2015-05-13 11:38 ` Segher Boessenkool
  0 siblings, 2 replies; 5+ messages in thread
From: Segher Boessenkool @ 2015-05-11 13:13 UTC (permalink / raw)
  To: gcc-patches; +Cc: rth, Segher Boessenkool

There already was code to just use the original reg:CC, but it was
positioned incorrectly: if the original code (that this RTL is
simplified from) did not already start with a COMPARE (or not in
the right mode), it didn't trigger.  But it is valid in that case
as well.

This then allows merging the other two arms of this conditional;
do so.

Bootstrapped and regression tested on powerpc64-linux and x86_64-linux;
no regressions.  Also built toolchains and Linux kernels for every
arch where that works; no new failures on that.

Any objections?


Segher


2015-05-11  Segher Boessenkool  <segher@kernel.crashing.org>

	* combine.c (simplify_set): When generating a CC set, if the
	source already is in the correct mode, do not wrap it in a
	compare.  Simplify the rest of that code.

---
 gcc/combine.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index 896d9d2..51f78a5 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -6675,20 +6675,16 @@ simplify_set (rtx x)
       if (other_changed)
 	undobuf.other_insn = other_insn;
 
-      /* Otherwise, if we didn't previously have a COMPARE in the
-	 correct mode, we need one.  */
-      if (GET_CODE (src) != COMPARE || GET_MODE (src) != compare_mode)
-	{
-	  SUBST (SET_SRC (x), gen_rtx_COMPARE (compare_mode, op0, op1));
-	  src = SET_SRC (x);
-	}
-      else if (GET_MODE (op0) == compare_mode && op1 == const0_rtx)
+      /* Don't generate a compare of a CC with 0, just use that CC.  */
+      if (GET_MODE (op0) == compare_mode && op1 == const0_rtx)
 	{
 	  SUBST (SET_SRC (x), op0);
 	  src = SET_SRC (x);
 	}
-      /* Otherwise, update the COMPARE if needed.  */
-      else if (XEXP (src, 0) != op0 || XEXP (src, 1) != op1)
+      /* Otherwise, if we didn't previously have the same COMPARE we
+	 want, create it from scratch.  */
+      else if (GET_CODE (src) != COMPARE || GET_MODE (src) != compare_mode
+	       || XEXP (src, 0) != op0 || XEXP (src, 1) != op1)
 	{
 	  SUBST (SET_SRC (x), gen_rtx_COMPARE (compare_mode, op0, op1));
 	  src = SET_SRC (x);
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0)))
  2015-05-11 13:13 [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0))) Segher Boessenkool
@ 2015-05-11 17:23 ` Jeff Law
  2015-05-11 21:11   ` Segher Boessenkool
  2015-05-13 11:38 ` Segher Boessenkool
  1 sibling, 1 reply; 5+ messages in thread
From: Jeff Law @ 2015-05-11 17:23 UTC (permalink / raw)
  To: Segher Boessenkool, gcc-patches; +Cc: rth

On 05/11/2015 07:13 AM, Segher Boessenkool wrote:
> There already was code to just use the original reg:CC, but it was
> positioned incorrectly: if the original code (that this RTL is
> simplified from) did not already start with a COMPARE (or not in
> the right mode), it didn't trigger.  But it is valid in that case
> as well.
>
> This then allows merging the other two arms of this conditional;
> do so.
>
> Bootstrapped and regression tested on powerpc64-linux and x86_64-linux;
> no regressions.  Also built toolchains and Linux kernels for every
> arch where that works; no new failures on that.
>
> Any objections?
>
>
> Segher
>
>
> 2015-05-11  Segher Boessenkool  <segher@kernel.crashing.org>
>
> 	* combine.c (simplify_set): When generating a CC set, if the
> 	source already is in the correct mode, do not wrap it in a
> 	compare.  Simplify the rest of that code.
Seems reasonable.  Might not hurt to do a little testing on a cc0 target 
though.

jeff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0)))
  2015-05-11 17:23 ` Jeff Law
@ 2015-05-11 21:11   ` Segher Boessenkool
  2015-05-11 21:16     ` Jeff Law
  0 siblings, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2015-05-11 21:11 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, rth

On Mon, May 11, 2015 at 11:23:47AM -0600, Jeff Law wrote:
> >	* combine.c (simplify_set): When generating a CC set, if the
> >	source already is in the correct mode, do not wrap it in a
> >	compare.  Simplify the rest of that code.
> Seems reasonable.  Might not hurt to do a little testing on a cc0 target 
> though.

I tested on mn10300, cris, and m68k.  On m68k it triggers while
building libgcc (for 040).  I verified the transform is correct.

I don't have a setup to actually bootstrap or regression check
any cc0 target.  A recurring theme :-(


Segher

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0)))
  2015-05-11 21:11   ` Segher Boessenkool
@ 2015-05-11 21:16     ` Jeff Law
  0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2015-05-11 21:16 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, rth

On 05/11/2015 03:11 PM, Segher Boessenkool wrote:
> On Mon, May 11, 2015 at 11:23:47AM -0600, Jeff Law wrote:
>>> 	* combine.c (simplify_set): When generating a CC set, if the
>>> 	source already is in the correct mode, do not wrap it in a
>>> 	compare.  Simplify the rest of that code.
>> Seems reasonable.  Might not hurt to do a little testing on a cc0 target
>> though.
>
> I tested on mn10300, cris, and m68k.  On m68k it triggers while
> building libgcc (for 040).  I verified the transform is correct.
Seems reasonable to me.

>
> I don't have a setup to actually bootstrap or regression check
> any cc0 target.  A recurring theme :-(
Understood.  As I mentioned in IRC and in another thread, it may be 
useful to set up an aranym instance for m68k bootstrap testing in the 
compile farm.

For other targets, building foo-elf in a single tree build with 
binutils, newlib and the simulator, then running the gcc testsuite with 
the simulator is often a good alternative.  Probably wise to run through 
this once or twice just to get familiar with it, then pull it out when 
really needed ;)

jeff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0)))
  2015-05-11 13:13 [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0))) Segher Boessenkool
  2015-05-11 17:23 ` Jeff Law
@ 2015-05-13 11:38 ` Segher Boessenkool
  1 sibling, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2015-05-13 11:38 UTC (permalink / raw)
  To: gcc-patches; +Cc: rth

On Mon, May 11, 2015 at 06:13:40AM -0700, Segher Boessenkool wrote:
> 	* combine.c (simplify_set): When generating a CC set, if the
> 	source already is in the correct mode, do not wrap it in a
> 	compare.  Simplify the rest of that code.

Committed now.


Segher

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-05-13 11:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-11 13:13 [PATCH] combine: Don't create (set (reg:CC) (compare (reg:CC) (const0))) Segher Boessenkool
2015-05-11 17:23 ` Jeff Law
2015-05-11 21:11   ` Segher Boessenkool
2015-05-11 21:16     ` Jeff Law
2015-05-13 11:38 ` 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).