public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR54555: Use strict_low_part for loading a constant only if it is cheaper
@ 2014-06-17  7:47 Andreas Schwab
  2014-06-17 18:23 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Schwab @ 2014-06-17  7:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: Kazu Hirata

Postreload may transform (set (REGX) (CONST_INT A)) ... (set (REGX)
(CONST_INT B)) to (set (REGX) (CONST_INT A)) ... (set (STRICT_LOW_PART
(REGX)) (CONST_INT B)), but it should do that only if the latter is
cheaper.  On m68k, a full word load of a small constant with moveq is
cheaper than doing a byte load with move.b.

Tested on m68k-suse-linux and x86_64-suse-linux.  In both cases the size
of cc1* becomes smaller with this change.

Andreas.

	PR rtl-optimization/54555
	* postreload.c (move2add_use_add2_insn): Only substitute
	STRICT_LOW_PART if it is cheaper.
---
 gcc/postreload.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gcc/postreload.c b/gcc/postreload.c
index 9d71649..89f0c84 100644
--- a/gcc/postreload.c
+++ b/gcc/postreload.c
@@ -1805,10 +1805,14 @@ move2add_use_add2_insn (rtx reg, rtx sym, rtx off, rtx insn)
 				   gen_rtx_STRICT_LOW_PART (VOIDmode,
 							    narrow_reg),
 				   narrow_src);
-		  changed = validate_change (insn, &PATTERN (insn),
-					     new_set, 0);
-		  if (changed)
-		    break;
+		  get_full_set_rtx_cost (new_set, &newcst);
+		  if (costs_lt_p (&newcst, &oldcst, speed))
+		    {
+		      changed = validate_change (insn, &PATTERN (insn),
+						 new_set, 0);
+		      if (changed)
+			break;
+		    }
 		}
 	    }
 	}
-- 
2.0.0

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

* Re: [PATCH] PR54555: Use strict_low_part for loading a constant only if it is cheaper
  2014-06-17  7:47 [PATCH] PR54555: Use strict_low_part for loading a constant only if it is cheaper Andreas Schwab
@ 2014-06-17 18:23 ` Jeff Law
  2014-06-18 10:37   ` Andreas Schwab
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2014-06-17 18:23 UTC (permalink / raw)
  To: Andreas Schwab, gcc-patches; +Cc: Kazu Hirata

On 06/17/14 01:47, Andreas Schwab wrote:
> Postreload may transform (set (REGX) (CONST_INT A)) ... (set (REGX)
> (CONST_INT B)) to (set (REGX) (CONST_INT A)) ... (set (STRICT_LOW_PART
> (REGX)) (CONST_INT B)), but it should do that only if the latter is
> cheaper.  On m68k, a full word load of a small constant with moveq is
> cheaper than doing a byte load with move.b.
>
> Tested on m68k-suse-linux and x86_64-suse-linux.  In both cases the size
> of cc1* becomes smaller with this change.
>
> Andreas.
>
> 	PR rtl-optimization/54555
> 	* postreload.c (move2add_use_add2_insn): Only substitute
> 	STRICT_LOW_PART if it is cheaper.
Sadly, Kazu didn't add a testcase for the H8/300 cases which inspired 
his change, so we don't know if your patch hurts the H8/300 port or not.

Let's do better this time ;-)  Add a testcase for the m68k port which 
verifies we're getting the desired code.  I don't care if you test the 
assembly code or test the RTL dumps, just that we have a test for the 
case where STRICT_LOW_PART is not a win.

With a testcase, this is approved.

Thanks,

jeff

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

* Re: [PATCH] PR54555: Use strict_low_part for loading a constant only if it is cheaper
  2014-06-17 18:23 ` Jeff Law
@ 2014-06-18 10:37   ` Andreas Schwab
  0 siblings, 0 replies; 3+ messages in thread
From: Andreas Schwab @ 2014-06-18 10:37 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

Jeff Law <law@redhat.com> writes:

> Let's do better this time ;-)  Add a testcase for the m68k port which
> verifies we're getting the desired code.

Make sense.  Installed with the following test case.

Andreas.

	PR rtl-optimization/54555
	* gcc.target/m68k/pr54555.c: New test.

diff --git a/gcc/testsuite/gcc.target/m68k/pr54555.c b/gcc/testsuite/gcc.target/m68k/pr54555.c
new file mode 100644
index 0000000..4be704b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/m68k/pr54555.c
@@ -0,0 +1,13 @@
+/* PR rtl-optimization/54555
+   Test that postreload does not shorten the load of small constants to
+   use move.b instead of moveq.  */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "move\\.?b" } } */
+
+void foo (void);
+void bar (int a)
+{
+  if (a == 16 || a == 23) foo ();
+  if (a == -110 || a == -128) foo ();
+}
-- 
2.0.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

end of thread, other threads:[~2014-06-18 10:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-17  7:47 [PATCH] PR54555: Use strict_low_part for loading a constant only if it is cheaper Andreas Schwab
2014-06-17 18:23 ` Jeff Law
2014-06-18 10:37   ` Andreas Schwab

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).