Re: New parameters to control stringop expansion libcall strategy
Is this version ok for trunk? thanks, David On Thu, Aug 8, 2013 at 9:31 AM, Xinliang David Li davi...@google.com wrote: Updated. thanks, David On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
Is this version ok for trunk? It looks resonable, but I still do not like much the removal of const for tables. Doing so will push them all into David Malcom's per-thread global universe. Currently the algorithm is selected based on cost-memset/cost-memcpy. Instead of removing the const of all the CPU tables, I would preffer introducing two readwrite global variables memset_algs/memcpy_algs and feed them with proper table at a time we set up ix86_tune_features. This has chance to do the right thing with optimize attribute specifying algorithms and with the longer term threading plan. Honza thanks, David On Thu, Aug 8, 2013 at 9:31 AM, Xinliang David Li davi...@google.com wrote: Updated. thanks, David On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
On Fri, Aug 9, 2013 at 11:33 AM, Jan Hubicka hubi...@ucw.cz wrote: Is this version ok for trunk? It looks resonable, but I still do not like much the removal of const for tables. Doing so will push them all into David Malcom's per-thread global universe. Currently the algorithm is selected based on cost-memset/cost-memcpy. Instead of removing the const of all the CPU tables, I would preffer introducing two readwrite global variables memset_algs/memcpy_algs and feed them with proper table at a time we set up ix86_tune_features. I can do that in this patch. In the future, when we need to do tunings for those constants, we can revisit it. thanks, David This has chance to do the right thing with optimize attribute specifying algorithms and with the longer term threading plan. Honza thanks, David On Thu, Aug 8, 2013 at 9:31 AM, Xinliang David Li davi...@google.com wrote: Updated. thanks, David On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
On Fri, Aug 9, 2013 at 11:33 AM, Jan Hubicka hubi...@ucw.cz wrote: Is this version ok for trunk? It looks resonable, but I still do not like much the removal of const for tables. Doing so will push them all into David Malcom's per-thread global universe. Currently the algorithm is selected based on cost-memset/cost-memcpy. Instead of removing the const of all the CPU tables, I would preffer introducing two readwrite global variables memset_algs/memcpy_algs and feed them with proper table at a time we set up ix86_tune_features. I can do that in this patch. In the future, when we need to do tunings for those constants, we can revisit it. Yep, I think we can follow same strategy and just move them to a global constant. Those are part of the context/universum since they will be user rewritable then. Thanks, the patch is OK with this change. Honza thanks, David This has chance to do the right thing with optimize attribute specifying algorithms and with the longer term threading plan. Honza thanks, David On Thu, Aug 8, 2013 at 9:31 AM, Xinliang David Li davi...@google.com wrote: Updated. thanks, David On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
Updated patch attached (fixed header, buffer overflow, and warning --
error problems).
Ok for trunk?
thanks,
David
On Wed, Aug 7, 2013 at 6:04 PM, Joseph S. Myers jos...@codesourcery.com wrote:
On Wed, 7 Aug 2013, Xinliang David Li wrote:
Why the exception? This should only be used on the host, not the target.
Sorry, I copied the boiler-plate header from i386.h -- is it wrong there too?
tm.h gets included in target code because we haven't finished separating
target macros used on the target from those used on the host.
--
Joseph S. Myers
jos...@codesourcery.com
Index: config/i386/stringop.def
===
--- config/i386/stringop.def(revision 0)
+++ config/i386/stringop.def(revision 0)
@@ -0,0 +1,37 @@
+/* Definitions for stringop strategy for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the files COPYING3. If not,
+see http://www.gnu.org/licenses/. */
+
+DEF_ENUM
+DEF_ALG (no_stringop, no_stringop)
+DEF_ENUM
+DEF_ALG (libcall, libcall)
+DEF_ENUM
+DEF_ALG (rep_prefix_1_byte, rep_byte)
+DEF_ENUM
+DEF_ALG (rep_prefix_4_byte, rep_4byte)
+DEF_ENUM
+DEF_ALG (rep_prefix_8_byte, rep_8byte)
+DEF_ENUM
+DEF_ALG (loop_1_byte, byte_loop)
+DEF_ENUM
+DEF_ALG (loop, loop)
+DEF_ENUM
+DEF_ALG (unrolled_loop, unrolled_loop)
+DEF_ENUM
+DEF_ALG (vector_loop, vector_loop)
Index: config/i386/stringop.opt
===
--- config/i386/stringop.opt(revision 0)
+++ config/i386/stringop.opt(revision 0)
@@ -0,0 +1,31 @@
+/* Definitions for stringop option handling for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the files COPYING3. If not,
+see http://www.gnu.org/licenses/. */
+
+Enum(stringop_alg) String(rep_byte) Value(rep_prefix_1_byte)
+
+#undef DEF_ENUM
+#define DEF_ENUM EnumValue
+
+#undef DEF_ALG
+#define DEF_ALG(alg, name) Enum(stringop_alg) String(name) Value(alg)
+
+#include stringop.def
+
+#undef DEF_ENUM
+#undef DEF_ALG
Index: config/i386/i386-opts.h
===
--- config/i386/i386-opts.h (revision 201581)
+++ config/i386/i386-opts.h (working copy)
@@ -28,15 +28,17 @@ see the files COPYING3 and COPYING.RUNTI
/* Algorithm to expand string function with. */
enum stringop_alg
{
- no_stringop,
- libcall,
- rep_prefix_1_byte,
- rep_prefix_4_byte,
- rep_prefix_8_byte,
- loop_1_byte,
- loop,
- unrolled_loop,
- vector_loop
+#undef DEF_ENUM
+#define DEF_ENUM
+
+#undef DEF_ALG
+#define DEF_ALG(alg, name) alg,
+
+#include stringop.def
+last_alg
+
+#undef DEF_ENUM
+#undef DEF_ALG
};
/* Available call abi. */
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 201582)
+++ config/i386/i386.c (working copy)
@@ -158,7 +158,7 @@ struct processor_costs ix86_size_cost =
};
/* Processor costs (relative to an add) */
-static const
+static
struct processor_costs i386_cost = { /* 386 specific costs */
COSTS_N_INSNS (1), /* cost of an add instruction */
COSTS_N_INSNS (1), /* cost of a lea instruction */
@@ -228,7 +228,7 @@ struct processor_costs i386_cost = {/*
1, /* cond_not_taken_branch_cost. */
};
-static const
+static
struct processor_costs i486_cost = { /* 486 specific costs */
COSTS_N_INSNS (1), /* cost of an add instruction */
COSTS_N_INSNS (1), /* cost of a lea instruction */
@@ -300,7 +300,7 @@ struct processor_costs i486_cost = {/*
1, /* cond_not_taken_branch_cost. */
};
-static const
+static
struct processor_costs pentium_cost = {
COSTS_N_INSNS (1), /* cost
Re: New parameters to control stringop expansion libcall strategy
On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
Updated.
thanks,
David
On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote:
On Wed, 7 Aug 2013, Xinliang David Li wrote:
Updated patch attached (fixed header, buffer overflow, and warning --
error problems).
You still have diagnostics starting with a capital letter, contrary to the
GNU Coding Standards.
--
Joseph S. Myers
jos...@codesourcery.com
Index: testsuite/gcc.target/i386/memcpy-strategy-1.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-1.c (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-1.c (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -march=atom -mmemcpy-strategy=vector_loop:-1:align } */
+/* { dg-final { scan-assembler-times movdqa 8 { target { ! { ia32 } } } } }
*/
+/* { dg-final { scan-assembler-times movdqa 4 { target { ia32 } } } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+ __builtin_memcpy (a, b, 2048);
+}
+
Index: testsuite/gcc.target/i386/memcpy-strategy-2.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-2.c (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-2.c (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -march=atom
-mmemcpy-strategy=vector_loop:3000:align,libcall:-1:align } */
+/* { dg-final { scan-assembler-times movdqa 8 { target { ! { ia32 } } } } }
*/
+/* { dg-final { scan-assembler-times movdqa 4 { target { ia32 } } } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+ __builtin_memcpy (a, b, 2048);
+}
+
Index: testsuite/gcc.target/i386/memset-strategy-1.c
===
--- testsuite/gcc.target/i386/memset-strategy-1.c (revision 0)
+++ testsuite/gcc.target/i386/memset-strategy-1.c (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -march=atom -mmemset-strategy=libcall:-1:align } */
+/* { dg-final { scan-assembler-times memset 2 } } */
+
+char a[2048];
+void t (void)
+{
+ __builtin_memset (a, 1, 2048);
+}
+
Index: testsuite/gcc.target/i386/memcpy-strategy-3.c
===
--- testsuite/gcc.target/i386/memcpy-strategy-3.c (revision 0)
+++ testsuite/gcc.target/i386/memcpy-strategy-3.c (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -march=atom
-mmemcpy-strategy=vector_loop:2000:align,libcall:-1:align } */
+/* { dg-final { scan-assembler-times memcpy 2 } } */
+
+char a[2048];
+char b[2048];
+void t (void)
+{
+ __builtin_memcpy (a, b, 2048);
+}
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 201581)
+++ doc/invoke.texi (working copy)
@@ -652,6 +652,7 @@ Objective-C and Objective-C++ Dialects}.
-mbmi2 -mrtm -mlwp -mthreads @gol
-mno-align-stringops -minline-all-stringops @gol
-minline-stringops-dynamically -mstringop-strategy=@var{alg} @gol
+-mmemcpy-strategy=@var{strategy} -mmemset-strategy=@var{strategy}
-mpush-args -maccumulate-outgoing-args -m128bit-long-double @gol
-m96bit-long-double -mlong-double-64 -mlong-double-80 @gol
-mregparm=@var{num} -msseregparm @gol
@@ -14651,6 +14652,24 @@ Expand into an inline loop.
Always use a library call.
@end table
+@item -mmemcpy-strategy=@var{strategy}
+@opindex mmemcpy-strategy=@var{strategy}
+Override the internal decision heuristic to decide if @code{__builtin_memcpy}
+should be inlined and what inline algorithm to use when the expected size
+of the copy operation is known. @var{strategy}
+is a comma-separated list of @var{alg}:@var{max_size}:@var{dest_align}
triplets.
+@var{alg} is specified in @option{-mstringop-strategy}, @var{max_size}
specifies
+the max byte size with which inline algorithm @var{alg} is allowed. For the
last
+triplet, the @var{max_size} must be @code{-1}. The @var{max_size} of the
triplets
+in the list must be specified in increasing order. The minimal byte size for
+@var{alg} is @code{0} for the first triplet and @code{@var{max_size} + 1} of
the
+preceding range.
+
+@item -mmemset-strategy=@var{strategy}
+@opindex mmemset-strategy=@var{strategy}
+The option is similar to @option{-mmemcpy-strategy=} except that it is to
control
+@code{__builtin_memset} expansion.
+
@item -momit-leaf-frame-pointer
@opindex momit-leaf-frame-pointer
Don't keep the frame pointer in a register for leaf functions. This
Index: config/i386/stringop.def
===
--- config/i386/stringop.def(revision 0)
+++ config/i386/stringop.def(revision 0)
@@ -0,0 +1,37 @@
+/* Definitions for stringop strategy for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General
Re: New parameters to control stringop expansion libcall strategy
the option is designed for purpose like this. That's great, thanks! Michael David On 6 August 2013 20:42, Xinliang David Li davi...@google.com wrote: Corrected two small problems reported by the style checker (The warnings about the EnumValue for options in stringopt.opt are not valid). On Tue, Aug 6, 2013 at 1:46 AM, Michael Zolotukhin michael.v.zolotuk...@gmail.com wrote: There are still some formatting issues (like 8 spaces instead of a tab, wrong indentation of do-loop and some other places) - to reveal some of them you could use contrib/check_GNU_style.sh script. But that was a nitpicking again:) Actually I wanted to ask whether you're going to use this option for some performance experiments involving memmov/memset - if so, probably you could tune existing cost-models as well? Is it possible? the option is designed for purpose like this. thanks, David Michael On 5 August 2013 20:44, Xinliang David Li davi...@google.com wrote: thanks. Updated patch attached. David On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin michael.v.zolotuk...@gmail.com wrote: Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte,
Re: New parameters to control stringop expansion libcall strategy
Fixed the do while formatting. Ok for trunk with this version? thanks, David On Tue, Aug 6, 2013 at 2:42 AM, Jan Hubicka hubi...@ucw.cz wrote: 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. The patch looks resonable to me in general. I wonder why we need to bring all the cost tables non-const instead of just having writable storage for the current strategy like we do with other flags anyway. Your strings are definitely more readable than the in-memory representation I came up with. Perhaps we can even turn the cost tables into strings for easier maintenance? I guess they are bit confusing for people not familiar with a code. Honza On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt (working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg) Init(no_stringop) Chose strategy to generate stringop using Index: config/i386/stringop.opt === --- config/i386/stringop.opt (revision 0) +++ config/i386/stringop.opt (revision 0) @@ -0,0 +1,36 @@ +/* Definitions for option handling
Re: New parameters to control stringop expansion libcall strategy
On Wed, 7 Aug 2013, Xinliang David Li wrote:
Index: config/i386/stringop.def
===
--- config/i386/stringop.def (revision 0)
+++ config/i386/stringop.def (revision 0)
@@ -0,0 +1,42 @@
+/* Definitions for option handling for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
Why the exception? This should only be used on the host, not the target.
+ do
+{
+ int mins, maxs;
+ stringop_alg alg;
+ char alg_name[128];
+ char align[16];
+
+ next_range_str = strchr (curr_range_str, ',');
+ if (next_range_str)
+*next_range_str++ = '\0';
+
+ if (3 != sscanf (curr_range_str, %[^:]:%d:%s, alg_name, maxs,
align))
This appears to introduce buffer overruns, which is never OK - whatever
the length of strings in the command-line arguments, you must not overflow
fixed-width buffers, so you must specify maximum field widths for the %[]
and %s.
+{
+ warning (0, Wrong arg %s to option %s, curr_range_str,
+ is_memset ? -mmemset_strategy= : -mmemcpy_strategy=);
+ return;
Invalid option arguments should be errors, not warnings, and diagnostics
should not start with a capital letter. Same applies to other diagnostics
here.
Index: config/i386/stringop.opt
===
--- config/i386/stringop.opt (revision 0)
+++ config/i386/stringop.opt (revision 0)
@@ -0,0 +1,36 @@
+/* Definitions for option handling for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
Again, why the exception?
--
Joseph S. Myers
jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
On Wed, Aug 7, 2013 at 5:23 PM, Joseph S. Myers jos...@codesourcery.com wrote:
On Wed, 7 Aug 2013, Xinliang David Li wrote:
Index: config/i386/stringop.def
===
--- config/i386/stringop.def (revision 0)
+++ config/i386/stringop.def (revision 0)
@@ -0,0 +1,42 @@
+/* Definitions for option handling for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
Why the exception? This should only be used on the host, not the target.
Sorry, I copied the boiler-plate header from i386.h -- is it wrong there too?
+ do
+{
+ int mins, maxs;
+ stringop_alg alg;
+ char alg_name[128];
+ char align[16];
+
+ next_range_str = strchr (curr_range_str, ',');
+ if (next_range_str)
+*next_range_str++ = '\0';
+
+ if (3 != sscanf (curr_range_str, %[^:]:%d:%s, alg_name, maxs,
align))
This appears to introduce buffer overruns, which is never OK - whatever
the length of strings in the command-line arguments, you must not overflow
fixed-width buffers, so you must specify maximum field widths for the %[]
and %s.
Ok will fix.
+{
+ warning (0, Wrong arg %s to option %s, curr_range_str,
+ is_memset ? -mmemset_strategy= : -mmemcpy_strategy=);
+ return;
Invalid option arguments should be errors, not warnings, and diagnostics
should not start with a capital letter. Same applies to other diagnostics
here.
Ok will fix.
Index: config/i386/stringop.opt
===
--- config/i386/stringop.opt (revision 0)
+++ config/i386/stringop.opt (revision 0)
@@ -0,0 +1,36 @@
+/* Definitions for option handling for IA-32.
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
Again, why the exception?
Wrong template used.
thanks,
David
--
Joseph S. Myers
jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
On Wed, 7 Aug 2013, Xinliang David Li wrote: Why the exception? This should only be used on the host, not the target. Sorry, I copied the boiler-plate header from i386.h -- is it wrong there too? tm.h gets included in target code because we haven't finished separating target macros used on the target from those used on the host. -- Joseph S. Myers jos...@codesourcery.com
Re: New parameters to control stringop expansion libcall strategy
There are still some formatting issues (like 8 spaces instead of a tab, wrong indentation of do-loop and some other places) - to reveal some of them you could use contrib/check_GNU_style.sh script. But that was a nitpicking again:) Actually I wanted to ask whether you're going to use this option for some performance experiments involving memmov/memset - if so, probably you could tune existing cost-models as well? Is it possible? Michael On 5 August 2013 20:44, Xinliang David Li davi...@google.com wrote: thanks. Updated patch attached. David On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin michael.v.zolotuk...@gmail.com wrote: Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt
Re: New parameters to control stringop expansion libcall strategy
2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. The patch looks resonable to me in general. I wonder why we need to bring all the cost tables non-const instead of just having writable storage for the current strategy like we do with other flags anyway. Your strings are definitely more readable than the in-memory representation I came up with. Perhaps we can even turn the cost tables into strings for easier maintenance? I guess they are bit confusing for people not familiar with a code. Honza On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt (working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg) Init(no_stringop) Chose strategy to generate stringop using Index: config/i386/stringop.opt === --- config/i386/stringop.opt (revision 0) +++ config/i386/stringop.opt (revision 0) @@ -0,0 +1,36 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it
Re: New parameters to control stringop expansion libcall strategy
On Tue, Aug 6, 2013 at 2:42 AM, Jan Hubicka hubi...@ucw.cz wrote: 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. The patch looks resonable to me in general. I wonder why we need to bring all the cost tables non-const instead of just having writable storage for the current strategy like we do with other flags anyway. Having const on those arrays do not bring us anything -- those tables will be accessed indirectly so const-prop won't happen anyways. current_strategy is an embedded struct in the cost array so it ends up in RO data when top level array is const. Your strings are definitely more readable than the in-memory representation I came up with. Perhaps we can even turn the cost tables into strings for easier maintenance? I guess they are bit confusing for people not familiar with a code. I think the in memory representation is fine -- if there is a need for internal representation cleanup, it should done as another patch. WDTY? thanks, David Honza On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt (working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg)
Re: New parameters to control stringop expansion libcall strategy
Corrected two small problems reported by the style checker (The warnings about the EnumValue for options in stringopt.opt are not valid). On Tue, Aug 6, 2013 at 1:46 AM, Michael Zolotukhin michael.v.zolotuk...@gmail.com wrote: There are still some formatting issues (like 8 spaces instead of a tab, wrong indentation of do-loop and some other places) - to reveal some of them you could use contrib/check_GNU_style.sh script. But that was a nitpicking again:) Actually I wanted to ask whether you're going to use this option for some performance experiments involving memmov/memset - if so, probably you could tune existing cost-models as well? Is it possible? the option is designed for purpose like this. thanks, David Michael On 5 August 2013 20:44, Xinliang David Li davi...@google.com wrote: thanks. Updated patch attached. David On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin michael.v.zolotuk...@gmail.com wrote: Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop)
Re: New parameters to control stringop expansion libcall strategy
Forgot the patch. David On Tue, Aug 6, 2013 at 9:42 AM, Xinliang David Li davi...@google.com wrote: Corrected two small problems reported by the style checker (The warnings about the EnumValue for options in stringopt.opt are not valid). On Tue, Aug 6, 2013 at 1:46 AM, Michael Zolotukhin michael.v.zolotuk...@gmail.com wrote: There are still some formatting issues (like 8 spaces instead of a tab, wrong indentation of do-loop and some other places) - to reveal some of them you could use contrib/check_GNU_style.sh script. But that was a nitpicking again:) Actually I wanted to ask whether you're going to use this option for some performance experiments involving memmov/memset - if so, probably you could tune existing cost-models as well? Is it possible? the option is designed for purpose like this. thanks, David Michael On 5 August 2013 20:44, Xinliang David Li davi...@google.com wrote: thanks. Updated patch attached. David On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin michael.v.zolotuk...@gmail.com wrote: Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte,
Re: New parameters to control stringop expansion libcall strategy
Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt (working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg) Init(no_stringop) Chose strategy to generate stringop using Index: config/i386/stringop.opt
Re: New parameters to control stringop expansion libcall strategy
thanks. Updated patch attached. David On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin michael.v.zolotuk...@gmail.com wrote: Hi, This is a really convenient option, thanks for working on it. I can't approve it as I'm not a maintainer, but it looks ok to me, except fot a small nitpicking: afair, comments should end with dot-space-space. Michael On 04 Aug 20:01, Xinliang David Li wrote: The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def (revision 0) +++ config/i386/stringop.def (revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt (revision 201458) +++ config/i386/i386.opt (working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg)
Re: New parameters to control stringop expansion libcall strategy
The attached is a new patch implementing the stringop inline strategy control using two new -m options: -mmemcpy-strategy= -mmemset-strategy= See changes in doc/invoke.texi for description of the new options. Example: -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned tells compiler to inline memcpy using rep_8byte when the size is no larger than 64 byte, using unrolled_loop when size is no larger than 2048, and for size 2048, using library call. In all cases, destination alignment adjustment is not done. Tested on x86-64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * config/i386/stringop.def: New file. * config/i386/stringop.opt: New file. * config/i386/i386-opts.h: Include stringopt.def. * config/i386/i386.opt: Include stringopt.opt. * config/i386/i386.c (ix86_option_override_internal): Override default size based stringop inline strategies with options. * config/i386/i386.c (ix86_parse_stringop_strategy_string): New function. 2013-08-04 Xinliang David Li davi...@google.com * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test. * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto. * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto. * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto. On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li davi...@google.com wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Index: config/i386/stringop.def === --- config/i386/stringop.def(revision 0) +++ config/i386/stringop.def(revision 0) @@ -0,0 +1,42 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/i386.opt === --- config/i386/i386.opt(revision 201458) +++ config/i386/i386.opt(working copy) @@ -316,6 +316,14 @@ mstack-arg-probe Target Report Mask(STACK_PROBE) Save Enable stack probing +mmemcpy-strategy= +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy) +Specify memcpy expansion strategy when expected size is known + +mmemset-strategy= +Target RejectNegative Joined Var(ix86_tune_memset_strategy) +Specify memset expansion strategy when expected size is known + mstringop-strategy= Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg) Init(no_stringop) Chose strategy to generate stringop using Index: config/i386/stringop.opt === --- config/i386/stringop.opt(revision 0) +++ config/i386/stringop.opt(revision 0) @@ -0,0 +1,36 @@ +/* Definitions for option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public
Backend specific params.def? (Was Re: New parameters to control stringop expansion libcall strategy)
On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Hi, problem with this is that we introduce generic --param that is used only by x86 backend. I am not really guru on the command line options, but I think this is first time we try to do such thing. I wonder if 1) We want to introduce target specific params.def 2) We want to use usual -msomething= options 3) We want to go this way? Honza
Re: Backend specific params.def? (Was Re: New parameters to control stringop expansion libcall strategy)
On Sat, Aug 3, 2013 at 1:06 AM, Jan Hubicka hubi...@ucw.cz wrote: On x86_64, when the expected size of memcpy/memset is known (e.g, with FDO), libcall strategy is used with the size is 8192. This value is hard coded, which makes it hard to do performance tuning. This patch adds two new parameters to do that. Potential usage includes per-application libcall strategy min-size tuning based on summary data with FDO (e.g, instruction workset size). Bootstrap and tested on x86_64/linux. Ok for trunk? thanks, David 2013-08-02 Xinliang David Li davi...@google.com * params.def: New parameters. * config/i386/i386.c (ix86_option_override_internal): Override default libcall size limit with parameters. Hi, problem with this is that we introduce generic --param that is used only by x86 backend. I am not really guru on the command line options, but I think this is first time we try to do such thing. I wonder if 1) We want to introduce target specific params.def We do have target specific tuning code for parameters though -- backend overrides the default value -- I think this is essentially target specific params. 2) We want to use usual -msomething= options 3) We want to go this way? I don't have strong opinion either way. To avoid controversy, let me work on a -mxxx= version of the patch -- and hopefully it will be more powerful. thanks, David Honza
