* [PATCH, rs6000] enable early debug and disable switch for gimple folding @ 2017-08-08 21:15 Will Schmidt 2017-08-08 22:31 ` Segher Boessenkool 2017-08-09 14:39 ` [PATCH, rs6000] (v2) " Will Schmidt 0 siblings, 2 replies; 6+ messages in thread From: Will Schmidt @ 2017-08-08 21:15 UTC (permalink / raw) To: GCC Patches Cc: Segher Boessenkool, Richard Biener, Bill Schmidt, David Edelsohn Hi, [PATCH, rs6000] enable early debug and disable switch for gimple folding Enable debug options related to gimple folding for the rs6000 target. Adds some output to the existing -mdebug=builtin option Add a -mgimple-folding=off option to disable the early rs6000 gimple folding. OK for trunk? Thanks, -Will [gcc] 2017-08-08 Will Schmidt <will_schmidt@vnet.ibm.com> * config/rs6000/rs6000.c: rs6000_option_override_internal() Add blurb to indicate when early gimple folding has been disabled. rs6000_gimple_fold_builtin(): Add debug content. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 1fb9861..0466fd0 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) { warning (0, N_("-maltivec=le not allowed for big-endian targets")); rs6000_altivec_element_order = 0; } + if (rs6000_gimple_folding_disable) + fprintf (stderr, + "gimple folding of rs6000 builtins has been disabled.\n"); + /* Add some warnings for VSX. */ if (TARGET_VSX) { const char *msg = NULL; if (!TARGET_HARD_FLOAT || !TARGET_SINGLE_FLOAT || !TARGET_DOUBLE_FLOAT) @@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD); enum rs6000_builtins fn_code = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl); tree arg0, arg1, lhs; + size_t uns_fncode = (size_t)fn_code; + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; + const char *fn_name2 = ((icode != CODE_FOR_nothing) + ? get_insn_name ((int)icode) + : "nothing"); + + if (TARGET_DEBUG_BUILTIN) + { + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s \n", + fn_code,fn_name1,fn_name2); + } + + if (rs6000_gimple_folding_disable) + return false; + /* Generic solution to prevent gimple folding of code without a LHS. */ if (!gimple_call_lhs (stmt)) return false; switch (fn_code) @@ -16516,10 +16536,13 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); update_call_from_tree (gsi, res); return true; } default: + if (TARGET_DEBUG_BUILTIN) + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", + fn_code,fn_name1,fn_name2); break; } return false; } diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt index e94aa07..4372b00 100644 --- a/gcc/config/rs6000/rs6000.opt +++ b/gcc/config/rs6000/rs6000.opt @@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian element order. maltivec=be Target Report RejectNegative Var(rs6000_altivec_element_order, 2) Generate AltiVec instructions using big-endian element order. +mgimple-folding=off +Target Report RejectNegative Var(rs6000_gimple_folding_disable, 1) +Disable early gimple folding of builtins. + mhard-dfp Target Report Mask(DFP) Var(rs6000_isa_flags) Use decimal floating point instructions. mmulhw ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] enable early debug and disable switch for gimple folding 2017-08-08 21:15 [PATCH, rs6000] enable early debug and disable switch for gimple folding Will Schmidt @ 2017-08-08 22:31 ` Segher Boessenkool 2017-08-09 14:40 ` Will Schmidt 2017-08-09 14:39 ` [PATCH, rs6000] (v2) " Will Schmidt 1 sibling, 1 reply; 6+ messages in thread From: Segher Boessenkool @ 2017-08-08 22:31 UTC (permalink / raw) To: Will Schmidt; +Cc: GCC Patches, Richard Biener, Bill Schmidt, David Edelsohn Hi! On Tue, Aug 08, 2017 at 04:14:56PM -0500, Will Schmidt wrote: > * config/rs6000/rs6000.c: rs6000_option_override_internal() Add blurb > to indicate when early gimple folding has been disabled. * config/rs6000/rs6000.c (rs6000_option_override_internal): ... > rs6000_gimple_fold_builtin(): Add debug content. (rs6000_gimple_fold_builtin): ... > @@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) > gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD); > enum rs6000_builtins fn_code > = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl); > tree arg0, arg1, lhs; > > + size_t uns_fncode = (size_t)fn_code; Space after cast. More of this in the rest of the patch. > + if (TARGET_DEBUG_BUILTIN) > + { > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s \n", > + fn_code,fn_name1,fn_name2); No space before \n, space after commas. > + if (rs6000_gimple_folding_disable) > + return false; One space indented too many there. > diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt > index e94aa07..4372b00 100644 > --- a/gcc/config/rs6000/rs6000.opt > +++ b/gcc/config/rs6000/rs6000.opt > @@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian element order. > > maltivec=be > Target Report RejectNegative Var(rs6000_altivec_element_order, 2) > Generate AltiVec instructions using big-endian element order. > > +mgimple-folding=off > +Target Report RejectNegative Var(rs6000_gimple_folding_disable, 1) > +Disable early gimple folding of builtins. Please use -mgimple-folding instead, or better, -mfold-gimple, along with its no- variant? So no RejectNegative. It's probably easier to read if the internal var is the positive version, too? Segher ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] enable early debug and disable switch for gimple folding 2017-08-08 22:31 ` Segher Boessenkool @ 2017-08-09 14:40 ` Will Schmidt 0 siblings, 0 replies; 6+ messages in thread From: Will Schmidt @ 2017-08-09 14:40 UTC (permalink / raw) To: Segher Boessenkool Cc: GCC Patches, Richard Biener, Bill Schmidt, David Edelsohn On Tue, 2017-08-08 at 17:31 -0500, Segher Boessenkool wrote: > Hi! > > On Tue, Aug 08, 2017 at 04:14:56PM -0500, Will Schmidt wrote: > > * config/rs6000/rs6000.c: rs6000_option_override_internal() Add blurb > > to indicate when early gimple folding has been disabled. > > * config/rs6000/rs6000.c (rs6000_option_override_internal): ... > > > > rs6000_gimple_fold_builtin(): Add debug content. > > (rs6000_gimple_fold_builtin): ... > > > @@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) > > gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD); > > enum rs6000_builtins fn_code > > = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl); > > tree arg0, arg1, lhs; > > > > + size_t uns_fncode = (size_t)fn_code; > > Space after cast. More of this in the rest of the patch. > > > + if (TARGET_DEBUG_BUILTIN) > > + { > > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s \n", > > + fn_code,fn_name1,fn_name2); > > No space before \n, space after commas. > > > + if (rs6000_gimple_folding_disable) > > + return false; > > One space indented too many there. > > > diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt > > index e94aa07..4372b00 100644 > > --- a/gcc/config/rs6000/rs6000.opt > > +++ b/gcc/config/rs6000/rs6000.opt > > @@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian element order. > > > > maltivec=be > > Target Report RejectNegative Var(rs6000_altivec_element_order, 2) > > Generate AltiVec instructions using big-endian element order. > > > > +mgimple-folding=off > > +Target Report RejectNegative Var(rs6000_gimple_folding_disable, 1) > > +Disable early gimple folding of builtins. > > Please use -mgimple-folding instead, or better, -mfold-gimple, along > with its no- variant? So no RejectNegative. > > It's probably easier to read if the internal var is the positive > version, too? Thanks for the review. (v2) incoming momentarily. > > > Segher > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding 2017-08-08 21:15 [PATCH, rs6000] enable early debug and disable switch for gimple folding Will Schmidt 2017-08-08 22:31 ` Segher Boessenkool @ 2017-08-09 14:39 ` Will Schmidt 2017-08-09 21:06 ` Segher Boessenkool 1 sibling, 1 reply; 6+ messages in thread From: Will Schmidt @ 2017-08-09 14:39 UTC (permalink / raw) To: GCC Patches Cc: Segher Boessenkool, Richard Biener, Bill Schmidt, David Edelsohn Hi, [Patch, rs6000] (v2) enable early debug and disable switch for gimple folding Enable debug options related to gimple folding for the rs6000 target. Adds some output to the existing -mdebug=builtin option Add a -mgimple-folding=off option to disable the early rs6000 gimple folding. (V2 updates) Changed variable name to rs6000_fold_gimple. Changed option name to "fold-gimple", so -mfold-gimple and -mno-fold-gimple options both work smoothly. Whitespace updates as noted per review. I also fixed the (missing) space after cast for the existing debug code. OK for trunk? Thanks, -Will [gcc] 2017-08-09 Will Schmidt <will_schmidt@vnet.ibm.com> * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb to indicate when early gimple folding has been disabled. (rs6000_gimple_fold_builtin): Add debug content. (rs6000_invalid_builtin): whitespace. (rs6000_expand_builtin): Whitespace. * config/rs6000/rs6000.opt: Add option for -mfold-gimple. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 1fb9861..f970f9e 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) { warning (0, N_("-maltivec=le not allowed for big-endian targets")); rs6000_altivec_element_order = 0; } + if (!rs6000_fold_gimple) + fprintf (stderr, + "gimple folding of rs6000 builtins has been disabled.\n"); + /* Add some warnings for VSX. */ if (TARGET_VSX) { const char *msg = NULL; if (!TARGET_HARD_FLOAT || !TARGET_SINGLE_FLOAT || !TARGET_DOUBLE_FLOAT) @@ -16052,11 +16056,11 @@ paired_expand_predicate_builtin (enum insn_code icode, tree exp, rtx target) appropriate target options being set. */ static void rs6000_invalid_builtin (enum rs6000_builtins fncode) { - size_t uns_fncode = (size_t)fncode; + size_t uns_fncode = (size_t) fncode; const char *name = rs6000_builtin_info[uns_fncode].name; HOST_WIDE_INT fnmask = rs6000_builtin_info[uns_fncode].mask; gcc_assert (name != NULL); if ((fnmask & RS6000_BTM_CELL) != 0) @@ -16157,10 +16161,26 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) gcc_checking_assert (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD); enum rs6000_builtins fn_code = (enum rs6000_builtins) DECL_FUNCTION_CODE (fndecl); tree arg0, arg1, lhs; + size_t uns_fncode = (size_t) fn_code; + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; + const char *fn_name2 = ((icode != CODE_FOR_nothing) + ? get_insn_name ((int) icode) + : "nothing"); + + if (TARGET_DEBUG_BUILTIN) + { + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n", + fn_code, fn_name1, fn_name2); + } + + if (!rs6000_fold_gimple) + return false; + /* Generic solution to prevent gimple folding of code without a LHS. */ if (!gimple_call_lhs (stmt)) return false; switch (fn_code) @@ -16516,10 +16536,13 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); update_call_from_tree (gsi, res); return true; } default: + if (TARGET_DEBUG_BUILTIN) + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", + fn_code, fn_name1, fn_name2); break; } return false; } @@ -16549,11 +16572,11 @@ rs6000_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED, if (TARGET_DEBUG_BUILTIN) { enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; const char *name1 = rs6000_builtin_info[uns_fcode].name; const char *name2 = ((icode != CODE_FOR_nothing) - ? get_insn_name ((int)icode) + ? get_insn_name ((int) icode) : "nothing"); const char *name3; switch (rs6000_builtin_info[uns_fcode].attr & RS6000_BTC_TYPE_MASK) { @@ -16569,11 +16592,11 @@ rs6000_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED, fprintf (stderr, "rs6000_expand_builtin, %s (%d), insn = %s (%d), type=%s%s\n", (name1) ? name1 : "---", fcode, - (name2) ? name2 : "---", (int)icode, + (name2) ? name2 : "---", (int) icode, name3, func_valid_p ? "" : ", not valid"); } if (!func_valid_p) diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt index e94aa07..65e4724 100644 --- a/gcc/config/rs6000/rs6000.opt +++ b/gcc/config/rs6000/rs6000.opt @@ -146,10 +146,14 @@ Generate AltiVec instructions using little-endian element order. maltivec=be Target Report RejectNegative Var(rs6000_altivec_element_order, 2) Generate AltiVec instructions using big-endian element order. +mfold-gimple +Target Report Var(rs6000_fold_gimple, 1) Init(1) +Enable early gimple folding of builtins. + mhard-dfp Target Report Mask(DFP) Var(rs6000_isa_flags) Use decimal floating point instructions. mmulhw ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding 2017-08-09 14:39 ` [PATCH, rs6000] (v2) " Will Schmidt @ 2017-08-09 21:06 ` Segher Boessenkool 2017-08-10 0:04 ` Segher Boessenkool 0 siblings, 1 reply; 6+ messages in thread From: Segher Boessenkool @ 2017-08-09 21:06 UTC (permalink / raw) To: Will Schmidt; +Cc: GCC Patches, Richard Biener, Bill Schmidt, David Edelsohn On Wed, Aug 09, 2017 at 09:39:08AM -0500, Will Schmidt wrote: > I also fixed the (missing) space after > cast for the existing debug code. Thanks for that :-) > * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb > to indicate when early gimple folding has been disabled. The colon goes after the closing parenthesis. > (rs6000_invalid_builtin): whitespace. Capital W. Or, say what this does? "Fix whitespace." > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 1fb9861..f970f9e 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) > { > warning (0, N_("-maltivec=le not allowed for big-endian targets")); > rs6000_altivec_element_order = 0; > } > > + if (!rs6000_fold_gimple) > + fprintf (stderr, > + "gimple folding of rs6000 builtins has been disabled.\n"); That first " should line up with the s in stderr. Should this print a message though? > + size_t uns_fncode = (size_t) fn_code; > + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; > + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; > + const char *fn_name2 = ((icode != CODE_FOR_nothing) > + ? get_insn_name ((int) icode) > + : "nothing"); Similarly, the ? and : should line up with the second (. (You can also remove the superfluous outer parens). > + if (TARGET_DEBUG_BUILTIN) > + { > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n", > + fn_code, fn_name1, fn_name2); > + } Wrong indent on the {}; but you don't need those here anyway, it's a single statement. > default: > + if (TARGET_DEBUG_BUILTIN) > + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", > + fn_code, fn_name1, fn_name2); No space before \n please. > +mfold-gimple > +Target Report Var(rs6000_fold_gimple, 1) Init(1) > +Enable early gimple folding of builtins. You can drop the ", 1" I think? Okay with those nits fixed. Segher ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding 2017-08-09 21:06 ` Segher Boessenkool @ 2017-08-10 0:04 ` Segher Boessenkool 0 siblings, 0 replies; 6+ messages in thread From: Segher Boessenkool @ 2017-08-10 0:04 UTC (permalink / raw) To: Will Schmidt; +Cc: GCC Patches, Richard Biener, Bill Schmidt, David Edelsohn On Wed, Aug 09, 2017 at 09:39:08AM -0500, Will Schmidt wrote: > I also fixed the (missing) space after > cast for the existing debug code. Thanks for that :-) > * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb > to indicate when early gimple folding has been disabled. The colon goes after the closing parenthesis. > (rs6000_invalid_builtin): whitespace. Capital W. Or, say what this does? "Fix whitespace." > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 1fb9861..f970f9e 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) > { > warning (0, N_("-maltivec=le not allowed for big-endian targets")); > rs6000_altivec_element_order = 0; > } > > + if (!rs6000_fold_gimple) > + fprintf (stderr, > + "gimple folding of rs6000 builtins has been disabled.\n"); That first " should line up with the s in stderr. Should this print a message though? > + size_t uns_fncode = (size_t) fn_code; > + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; > + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; > + const char *fn_name2 = ((icode != CODE_FOR_nothing) > + ? get_insn_name ((int) icode) > + : "nothing"); Similarly, the ? and : should line up with the second (. (You can also remove the superfluous outer parens). > + if (TARGET_DEBUG_BUILTIN) > + { > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n", > + fn_code, fn_name1, fn_name2); > + } Wrong indent on the {}; but you don't need those here anyway, it's a single statement. > default: > + if (TARGET_DEBUG_BUILTIN) > + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", > + fn_code, fn_name1, fn_name2); No space before \n please. > +mfold-gimple > +Target Report Var(rs6000_fold_gimple, 1) Init(1) > +Enable early gimple folding of builtins. You can drop the ", 1" I think? Okay with those nits fixed. Segher ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-09 21:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-08 21:15 [PATCH, rs6000] enable early debug and disable switch for gimple folding Will Schmidt 2017-08-08 22:31 ` Segher Boessenkool 2017-08-09 14:40 ` Will Schmidt 2017-08-09 14:39 ` [PATCH, rs6000] (v2) " Will Schmidt 2017-08-09 21:06 ` Segher Boessenkool 2017-08-10 0:04 ` 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).