Ping: [PATCH] PR 48175, Make CASE_VALUES_THRESHOLD settable via --param
On Thu, Apr 21, 2011 at 03:02:10PM -0400, Michael Meissner wrote: In looking at some improvements to the powerpc, we wanted to change the default for when a table jump is generated vs. a series of if statements. Now, we could just add a powerpc specific TARGET_CASE_VALUES_THRESHOLD, but I tend to think that these should be settable on all/most ports with --param. At present, there are only two ports (avr and mn10300) that define their own TARGET_CASE_VALUES_THRESHOLD hook. My first patch does not remove the target hook and modify the avr/mn10300 ports to use maybe_set_param_value, but that can be done if desired. The patch adds two --param values, one for when the port is using the casesi insn, and the other when it uses the more primitive tablejump insn. I have bootstrapped the compiler with this patch and run the test suite with no regressions. Is it ok to apply as is? Should I modify the avr and mn10300 ports to use the parameters and do away with the target hook? Or should I do this just as a powerpc target hook? I never got a response for this, and my earlier ping didn't seem to go out. I'll check it in on Monday if there are no objections. -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Re: Ping: [PATCH] PR 48175, Make CASE_VALUES_THRESHOLD settable via --param
On Fri, May 06, 2011 at 12:21:24PM -0400, Michael Meissner wrote: On Thu, Apr 21, 2011 at 03:02:10PM -0400, Michael Meissner wrote: In looking at some improvements to the powerpc, we wanted to change the default for when a table jump is generated vs. a series of if statements. Now, we could just add a powerpc specific TARGET_CASE_VALUES_THRESHOLD, but I tend to think that these should be settable on all/most ports with --param. At present, there are only two ports (avr and mn10300) that define their own TARGET_CASE_VALUES_THRESHOLD hook. My first patch does not remove the target hook and modify the avr/mn10300 ports to use maybe_set_param_value, but that can be done if desired. The patch adds two --param values, one for when the port is using the casesi insn, and the other when it uses the more primitive tablejump insn. I have bootstrapped the compiler with this patch and run the test suite with no regressions. Is it ok to apply as is? Should I modify the avr and mn10300 ports to use the parameters and do away with the target hook? Or should I do this just as a powerpc target hook? I never got a response for this, and my earlier ping didn't seem to go out. I'll check it in on Monday if there are no objections. I think it is very weird to have two different params, if we need any such param, there should be just one and its default value should depend on HAVE_casesi. Jakub
Re: Ping: [PATCH] PR 48175, Make CASE_VALUES_THRESHOLD settable via --param
On Fri, May 06, 2011 at 06:30:07PM +0200, Jakub Jelinek wrote: On Fri, May 06, 2011 at 12:21:24PM -0400, Michael Meissner wrote: On Thu, Apr 21, 2011 at 03:02:10PM -0400, Michael Meissner wrote: In looking at some improvements to the powerpc, we wanted to change the default for when a table jump is generated vs. a series of if statements. Now, we could just add a powerpc specific TARGET_CASE_VALUES_THRESHOLD, but I tend to think that these should be settable on all/most ports with --param. At present, there are only two ports (avr and mn10300) that define their own TARGET_CASE_VALUES_THRESHOLD hook. My first patch does not remove the target hook and modify the avr/mn10300 ports to use maybe_set_param_value, but that can be done if desired. The patch adds two --param values, one for when the port is using the casesi insn, and the other when it uses the more primitive tablejump insn. I have bootstrapped the compiler with this patch and run the test suite with no regressions. Is it ok to apply as is? Should I modify the avr and mn10300 ports to use the parameters and do away with the target hook? Or should I do this just as a powerpc target hook? I never got a response for this, and my earlier ping didn't seem to go out. I'll check it in on Monday if there are no objections. I think it is very weird to have two different params, if we need any such param, there should be just one and its default value should depend on HAVE_casesi. The problem is the values in params.def must be constant, and can't depend on switches. I imagine we can have a single param that is normally 0, and if it is non-zero use that value, otherwise fall back to (HAVE_casesi ? 4 : 5). Or we could set it in finish_options in opts.c. Any preference? -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
[PATCH] PR 48175, Make CASE_VALUES_THRESHOLD settable via --param
In looking at some improvements to the powerpc, we wanted to change the default
for when a table jump is generated vs. a series of if statements. Now, we
could just add a powerpc specific TARGET_CASE_VALUES_THRESHOLD, but I tend to
think that these should be settable on all/most ports with --param.
At present, there are only two ports (avr and mn10300) that define their own
TARGET_CASE_VALUES_THRESHOLD hook. My first patch does not remove the target
hook and modify the avr/mn10300 ports to use maybe_set_param_value, but that
can be done if desired.
The patch adds two --param values, one for when the port is using the casesi
insn, and the other when it uses the more primitive tablejump insn.
I have bootstrapped the compiler with this patch and run the test suite with no
regressions. Is it ok to apply as is? Should I modify the avr and mn10300
ports to use the parameters and do away with the target hook? Or should I do
this just as a powerpc target hook?
[gcc]
2011-04-21 Michael Meissner meiss...@linux.vnet.ibm.com
PR rtl-optimization/48715
* params.def (PARAM_CASE_VALUES_THRESHOD_CASESI): New parameter.
(PARAM_CASE_VALUES_THRESHOLD_TABLEJUMP): Ditto.
* params.h (CASE_VALUES_THRESHOLD_CASESI): Define.
(CASE_VALUES_THRESHOLD_TABLEJUMP): Ditto.
* targhooks.c (toplevel): Include params.h.
(default_case_values_threshold): Use CASE_VALUES_THRESHOLD_CASESI
and CASE_VALUES_THRESHOLD_TABLEJUMP.
* Makefile.in (targhooks.o): Add $(PARAMS_H) dependency.
* doc/invoke.texi (case-values-threshold-casesi): Document.
(case-values-threshold-tablejump): Ditto.
[gcc/testsuite]
2011-04-21 Michael Meissner meiss...@linux.vnet.ibm.com
PR rtl-optimization/48715
* gcc.target/powerpc/case-threshold1.c: New file to test --param
case-values-threshold-tablejump.
* gcc.target/powerpc/case-threshold2.c: Ditto.
--
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Index: gcc/params.def
===
--- gcc/params.def (revision 172832)
+++ gcc/params.def (working copy)
@@ -885,6 +885,22 @@ DEFPARAM (PARAM_MAX_STORES_TO_SINK,
2, 0, 0)
+/* Threshold for using jump table vs. if statements if the machine supports a
+ CASESI instruction. */
+DEFPARAM (PARAM_CASE_VALUES_THRESHOLD_CASESI,
+ case-values-threshold-casesi,
+ Minimum number of case elements to use a jump table instead of
+ if statements if the machine supports a CASESI instruction,
+ 4, 2, 0)
+
+/* Threshold for using jump table vs. if statements if the machine does not
+ support a CASESI instruction. */
+DEFPARAM (PARAM_CASE_VALUES_THRESHOLD_TABLEJUMP,
+ case-values-threshold-tablejump,
+ Minimum number of case elements to use a jump table instead of
+ if statements if the machine does not support a CASESI instruction,
+ 5, 2, 0)
+
/*
Local variables:
mode:c
Index: gcc/params.h
===
--- gcc/params.h(revision 172832)
+++ gcc/params.h(working copy)
@@ -206,4 +206,8 @@ extern void init_param_values (int *para
PARAM_VALUE (PARAM_MIN_NONDEBUG_INSN_UID)
#define MAX_STORES_TO_SINK \
PARAM_VALUE (PARAM_MAX_STORES_TO_SINK)
+#define CASE_VALUES_THRESHOLD_CASESI \
+ PARAM_VALUE (PARAM_CASE_VALUES_THRESHOLD_CASESI)
+#define CASE_VALUES_THRESHOLD_TABLEJUMP \
+ PARAM_VALUE (PARAM_CASE_VALUES_THRESHOLD_TABLEJUMP)
#endif /* ! GCC_PARAMS_H */
Index: gcc/targhooks.c
===
--- gcc/targhooks.c (revision 172832)
+++ gcc/targhooks.c (working copy)
@@ -71,7 +71,7 @@ along with GCC; see the file COPYING3.
#include opts.h
#include tree-flow.h
#include tree-ssa-alias.h
-
+#include params.h
bool
default_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
@@ -1209,7 +1209,9 @@ default_target_can_inline_p (tree caller
unsigned int default_case_values_threshold (void)
{
- return (HAVE_casesi ? 4 : 5);
+ return (HAVE_casesi
+ ? CASE_VALUES_THRESHOLD_CASESI
+ : CASE_VALUES_THRESHOLD_TABLEJUMP);
}
bool
Index: gcc/Makefile.in
===
--- gcc/Makefile.in (revision 172832)
+++ gcc/Makefile.in (working copy)
@@ -2807,7 +2807,7 @@ opts-common.o : opts-common.c $(OPTS_H)
targhooks.o : targhooks.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TREE_H) \
$(EXPR_H) $(TM_H) $(RTL_H) $(TM_P_H) $(FUNCTION_H) output.h
$(DIAGNOSTIC_CORE_H) \
$(MACHMODE_H) $(TARGET_DEF_H) $(TARGET_H) $(GGC_H) gt-targhooks.h \
- $(OPTABS_H) $(RECOG_H) reload.h hard-reg-set.h intl.h $(OPTS_H) \
+ $(OPTABS_H) $(RECOG_H) reload.h hard-reg-set.h intl.h $(OPTS_H) $(PARAMS_H)
\
tree-ssa-alias.h
