This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, SH] Add support for inlined builtin-strcmp (2/2)
- From: Oleg Endo <oleg dot endo at t-online dot de>
- To: Christian Bruel <christian dot bruel at st dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Kaz Kojima <kkojima at rr dot iij4u dot or dot jp>
- Date: Sat, 19 Oct 2013 11:30:42 +0200
- Subject: Re: [PATCH, SH] Add support for inlined builtin-strcmp (2/2)
- Authentication-results: sourceware.org; auth=none
- References: <525FF0F9 dot 4010704 at st dot com> <1382051119 dot 2445 dot 62 dot camel at yam-132-YW-E178-FTW> <5260EA68 dot 9080603 at st dot com>
On Fri, 2013-10-18 at 09:59 +0200, Christian Bruel wrote:
> On 10/18/2013 01:05 AM, Oleg Endo wrote:
> > I was wondering, in file sh-mem.c, the new function
> > 'sh4_expand_cmpstr' ... why is it SH4-something? It's a bit confusing,
> > since cmp/str has been around since ever (i.e. since SH1). Maybe just
> > rename it to 'sh_expand_cmpstr' instead?
>
> Just historical. (SH4* are our primary SH platforms). The code is
> enabled/tested for all SH1 of course, I will rename. Thanks .
>
> > Maybe just
> > rename it to 'sh_expand_cmpstr' instead? The function always returns
> > 'true', so maybe just make it return 'void'?
>
> yes, it's for genericity as I plan to reuse/specialize the code based on
> the count parameter for strncmp to be contributed next.
I already assumed so :)
> >
> > Also, in the expander ...
> >
> > + [(set (match_operand:SI 0 "register_operand" "")
> > + (compare:SI (match_operand:BLK 1 "memory_operand" "")
> >
> > ... no need to use empty "" constraints
>
> OK, thanks
Could you also please remove the quotes around the preparation block:
"
{
if (! optimize_insn_for_size_p () && sh4_expand_cmpstr(operands))
DONE;
else FAIL;
}")
I've attached two test cases, tested with
make -k check-gcc RUNTESTFLAGS="sh.exp=strcmp* --target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
Could you please include them?
Cheers,
Oleg
Index: gcc/testsuite/gcc.target/sh/strcmp-2.c
===================================================================
--- gcc/testsuite/gcc.target/sh/strcmp-2.c (revision 0)
+++ gcc/testsuite/gcc.target/sh/strcmp-2.c (revision 0)
@@ -0,0 +1,13 @@
+/* Check that the __builtin_strcmp function is not inlined when optimizing
+ for size. */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler-not "cmp/str" } } */
+/* { dg-final { scan-assembler "jsr|jmp|bsr|bra" } } */
+
+int
+test00 (const char* a, const char* b)
+{
+ return __builtin_strcmp (a, b);
+}
Index: gcc/testsuite/gcc.target/sh/strcmp-1.c
===================================================================
--- gcc/testsuite/gcc.target/sh/strcmp-1.c (revision 0)
+++ gcc/testsuite/gcc.target/sh/strcmp-1.c (revision 0)
@@ -0,0 +1,12 @@
+/* Check that the __builtin_strcmp function is inlined utilizing cmp/str insn
+ when optimizing for speed. */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler "cmp/str" } } */
+
+int
+test00 (const char* a, const char* b)
+{
+ return __builtin_strcmp (a, b);
+}