[Committed] S/390: Fix PR84332 ICE with stack clash protection
Our implementation of the stack probe requires the probe interval to be used as displacement in an address operand. The maximum probe interval currently is 64k. This would exceed short displacements. Trim that value down to 4k if that happens. This might result in too many probes being generated only on the oldest supported machine level z900. Bootstrapped and regression tested on s390x with z900 and z13 defaults. gcc/ChangeLog: 2018-08-09 Andreas Krebbel PR target/84332 * config/s390/s390.c (s390_option_override_internal): Reduce the stack-clash-protection-probe-interval param if it would be too big for z900. gcc/testsuite/ChangeLog: 2018-08-09 Andreas Krebbel PR target/84332 * gcc.target/s390/pr84332.c: New testcase. diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c index d533a3fcb4e..d5511d01192 100644 --- a/gcc/config/s390/s390.c +++ b/gcc/config/s390/s390.c @@ -14983,6 +14983,17 @@ s390_option_override_internal (struct gcc_options *opts, else if (opts->x_s390_stack_guard) error ("-mstack-guard implies use of -mstack-size"); + /* Our implementation of the stack probe requires the probe interval + to be used as displacement in an address operand. The maximum + probe interval currently is 64k. This would exceed short + displacements. Trim that value down to 4k if that happens. This + might result in too many probes being generated only on the + oldest supported machine level z900. */ + if (!DISP_IN_RANGE ((1 << PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL +set_param_value ("stack-clash-protection-probe-interval", 12, +opts->x_param_values, +opts_set->x_param_values); + #ifdef TARGET_DEFAULT_LONG_DOUBLE_128 if (!TARGET_LONG_DOUBLE_128_P (opts_set->x_target_flags)) opts->x_target_flags |= MASK_LONG_DOUBLE_128; diff --git a/gcc/testsuite/gcc.target/s390/pr84332.c b/gcc/testsuite/gcc.target/s390/pr84332.c new file mode 100644 index 000..3dec99f62c7 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/pr84332.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-march=z900 -fstack-clash-protection --param stack-clash-protection-probe-interval=16" } */ + +struct b +{ + char a[65536]; +}; + +void c (void) { struct b d; }
[PATCH][OBVIOUS] Fix typos in params.def.
Hi. It's quite obvious fix, fixes following param explanations: The minimum stride ratio for loop interchange to be profitable size of tiles for loop blocking. maximum number of parameters in a SCoP. maximum number of arrays per scop. maximum number of isl operations, 0 means unlimited maximum number of isl operations, 0 means unlimited whether codegen errors should be ICEs when -fchecking. use internal function id in profile lookup. track topn target addresses in indirect-call profile. Level of hsa debug stores verbosity Maximum number of may-defs visited when devirtualizing speculatively Maximum number of assertions to add along the default edge of a switch statement during VRP I'm going to install that. Martin gcc/ChangeLog: 2018-08-09 Martin Liska * params.def (PARAM_ALIGN_LOOP_ITERATIONS): Remove double dots at the end of a line, make first letter capital and end up a sentence with a dot. (PARAM_LOOP_INTERCHANGE_STRIDE_RATIO): Likewise. (PARAM_LOOP_BLOCK_TILE_SIZE): Likewise. (PARAM_GRAPHITE_MAX_NB_SCOP_PARAMS): Likewise. (PARAM_GRAPHITE_MAX_ARRAYS_PER_SCOP): Likewise. (PARAM_MAX_ISL_OPERATIONS): Likewise. (PARAM_GRAPHITE_ALLOW_CODEGEN_ERRORS): Likewise. (PARAM_PROFILE_FUNC_INTERNAL_ID): Likewise. (PARAM_INDIR_CALL_TOPN_PROFILE): Likewise. (PARAM_SLP_MAX_INSNS_IN_BB): Likewise. (PARAM_IPA_CP_EVAL_THRESHOLD): Likewise. (PARAM_IPA_CP_RECURSION_PENALTY): Likewise. (PARAM_IPA_CP_SINGLE_CALL_PENALTY): Likewise. (PARAM_IPA_CP_LOOP_HINT_BONUS): Likewise. (PARAM_IPA_CP_ARRAY_INDEX_HINT_BONUS): Likewise. (PARAM_TREE_REASSOC_WIDTH): Likewise. (PARAM_HSA_GEN_DEBUG_STORES): Likewise. (PARAM_MAX_SPECULATIVE_DEVIRT_MAYDEFS): Likewise. (PARAM_MAX_VRP_SWITCH_ASSERTIONS): Likewise. --- gcc/params.def | 52 +- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/gcc/params.def b/gcc/params.def index 6bedeb5aa2b..a09785d4e07 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -415,8 +415,8 @@ DEFPARAM (PARAM_ALIGN_THRESHOLD, DEFPARAM (PARAM_ALIGN_LOOP_ITERATIONS, "align-loop-iterations", - "Loops iterating at least selected number of iterations will get loop alignment..", - 4, 0, 0) + "Loops iterating at least selected number of iterations will get " + "loop alignment.", 4, 0, 0) /* For guessed profiles, the loops having unknown number of iterations are predicted to iterate relatively few (10) times at average. @@ -828,7 +828,7 @@ DEFPARAM (PARAM_LOOP_INTERCHANGE_MAX_NUM_STMTS, DEFPARAM (PARAM_LOOP_INTERCHANGE_STRIDE_RATIO, "loop-interchange-stride-ratio", - "The minimum stride ratio for loop interchange to be profitable", + "The minimum stride ratio for loop interchange to be profitable.", 2, 0, 0) /* Whether we should use canonical types rather than deep "structural" @@ -906,32 +906,32 @@ DEFPARAM (PARAM_SWITCH_CONVERSION_BRANCH_RATIO, DEFPARAM (PARAM_LOOP_BLOCK_TILE_SIZE, "loop-block-tile-size", - "size of tiles for loop blocking.", + "Size of tiles for loop blocking.", 51, 0, 0) /* Maximal number of parameters that we allow in a SCoP. */ DEFPARAM (PARAM_GRAPHITE_MAX_NB_SCOP_PARAMS, "graphite-max-nb-scop-params", - "maximum number of parameters in a SCoP.", + "Maximum number of parameters in a SCoP.", 10, 0, 0) /* Maximal number of array references in a scop. */ DEFPARAM (PARAM_GRAPHITE_MAX_ARRAYS_PER_SCOP, "graphite-max-arrays-per-scop", - "maximum number of arrays per scop.", + "Maximum number of arrays per scop.", 100, 0, 0) DEFPARAM (PARAM_MAX_ISL_OPERATIONS, "max-isl-operations", - "maximum number of isl operations, 0 means unlimited", + "Maximum number of isl operations, 0 means unlimited.", 35, 0, 0) /* For testsuite purposes allow to check for codegen error handling. */ DEFPARAM (PARAM_GRAPHITE_ALLOW_CODEGEN_ERRORS, "graphite-allow-codegen-errors", - "whether codegen errors should be ICEs when -fchecking.", + "Whether codegen errors should be ICEs when -fchecking.", 0, 0, 1) /* Avoid data dependence analysis on very large loops. */ @@ -951,23 +951,23 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, to look up for profile data. Otherwise, use a more stable external id based on assembler name and source location. */ DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID, - "profile-func-internal-id", - "use internal function id in profile lookup.", - 0, 0, 1) + "profile-func-internal-id", + "Use internal function id in profile lookup.", + 0, 0, 1) /* When the parameter is 1, track the most frequent N target addresses in indirect-call profile. This disables indirect_call_profiler_v2 which tracks single target. */ DEFPARAM (PARAM_INDIR_CALL_TOPN_PROFILE, - "indir-call-topn-profile", - "track
Re: [RFC][PATCH] Clean up of histogram allocation and release.
Ok, so I discussed that with Honza, the patch does not make sense any longer. Reason is that histograms are used in RTL expansion in order to provide hint for memcpy, memset-like operations. Please ignore the patch. Martin
Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib
On 09/08/18 06:56 +0200, Sebastian Huber wrote: On 08/08/18 16:33, Jonathan Wakely wrote: On 08/08/18 16:22 +0200, Ulrich Weigand wrote: Jonathan Wakely wrote: Aha, so newlib was using memalign previously: @@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz) #else extern "C" void *memalign(std::size_t boundary, std::size_t size); #endif -#define aligned_alloc memalign Yes, exactly ... this commit introduced the regression. OK, I've regressed the branches then - I'll fix that. This should fix it. I'll finish testing and commit it. Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for gcc-7-branch and gcc-8-branch, because changing newlib from using memalign to aligned_alloc is safe. Should I check in my patch in addition to your patch? Yes please, on trunk and 7 and 8. It's better to use the standard aligned_alloc if available.
[PATCH] Strip only selected predictors after early tree passes (PR tree-optimization/85799).
Hi. This patch adds an argument to the strip predictor pass. Early version strips only selected hints that are not reliable when inlining happens. The rest is striped in late tree passes. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Martin gcc/ChangeLog: 2018-08-09 Martin Liska PR tree-optimization/85799 * passes.def: Add new pass_strip_predict_hints_early. * predict.c (make_pass_profile): Add pass_data_strip_predict_hints_early and new pass pass_strip_predict_hints_early. (strip_predictor_early): New function. (pass_strip_predict_hints::execute): Call strip_predict_hints. (strip_predict_hints): New. (class pass_strip_predict_hints_early): New. (make_pass_strip_predict_hints_early): Likewise. * predict.def: Fix documentation. * tree-pass.h (make_pass_strip_predict_hints_early): New. gcc/testsuite/ChangeLog: 2018-08-09 Martin Liska * gcc.dg/pr85799.c: New test. --- gcc/passes.def | 2 +- gcc/predict.c | 138 - gcc/predict.def| 2 +- gcc/testsuite/gcc.dg/pr85799.c | 19 + gcc/tree-pass.h| 1 + 5 files changed, 123 insertions(+), 39 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr85799.c diff --git a/gcc/passes.def b/gcc/passes.def index 2a8fbc2efbe..3b1c15db1e1 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -98,7 +98,7 @@ along with GCC; see the file COPYING3. If not see early optimizations again. It is thus good idea to do this late. */ NEXT_PASS (pass_split_functions); - NEXT_PASS (pass_strip_predict_hints); + NEXT_PASS (pass_strip_predict_hints_early); POP_INSERT_PASSES () NEXT_PASS (pass_release_ssa_names); NEXT_PASS (pass_rebuild_cgraph_edges); diff --git a/gcc/predict.c b/gcc/predict.c index 96ae10f1319..a37c3845423 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -3878,44 +3878,27 @@ pass_profile::execute (function *fun) } // anon namespace -gimple_opt_pass * -make_pass_profile (gcc::context *ctxt) -{ - return new pass_profile (ctxt); -} - -namespace { - -const pass_data pass_data_strip_predict_hints = -{ - GIMPLE_PASS, /* type */ - "*strip_predict_hints", /* name */ - OPTGROUP_NONE, /* optinfo_flags */ - TV_BRANCH_PROB, /* tv_id */ - PROP_cfg, /* properties_required */ - 0, /* properties_provided */ - 0, /* properties_destroyed */ - 0, /* todo_flags_start */ - 0, /* todo_flags_finish */ -}; +/* Return true when PRED predictor should be removed after early + tree passes. */ -class pass_strip_predict_hints : public gimple_opt_pass +static bool +strip_predictor_early (enum br_predictor pred) { -public: - pass_strip_predict_hints (gcc::context *ctxt) -: gimple_opt_pass (pass_data_strip_predict_hints, ctxt) - {} - - /* opt_pass methods: */ - opt_pass * clone () { return new pass_strip_predict_hints (m_ctxt); } - virtual unsigned int execute (function *); - -}; // class pass_strip_predict_hints + switch (pred) +{ +case PRED_TREE_EARLY_RETURN: + return true; +default: + return false; +} +} /* Get rid of all builtin_expect calls and GIMPLE_PREDICT statements - we no longer need. */ + we no longer need. EARLY is set to true when called from early + optimizations. */ + unsigned int -pass_strip_predict_hints::execute (function *fun) +strip_predict_hints (function *fun, bool early) { basic_block bb; gimple *ass_stmt; @@ -3931,15 +3914,20 @@ pass_strip_predict_hints::execute (function *fun) if (gimple_code (stmt) == GIMPLE_PREDICT) { - gsi_remove (&bi, true); - changed = true; - continue; + if (!early + || strip_predictor_early (gimple_predict_predictor (stmt))) + { + gsi_remove (&bi, true); + changed = true; + continue; + } } else if (is_gimple_call (stmt)) { tree fndecl = gimple_call_fndecl (stmt); - if ((fndecl + if ((!early + && fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_EXPECT && gimple_call_num_args (stmt) == 2) @@ -3967,6 +3955,43 @@ pass_strip_predict_hints::execute (function *fun) return changed ? TODO_cleanup_cfg : 0; } +gimple_opt_pass * +make_pass_profile (gcc::context *ctxt) +{ + return new pass_profile (ctxt); +} + +namespace { + +const pass_data pass_data_strip_predict_hints = +{ + GIMPLE_PASS, /* type */ + "*strip_predict_hints", /* name */ + OPTGROUP_NONE, /* optinfo_flags */ + TV_BRANCH_PROB, /* tv_id */ + PROP_cfg, /* properties_required */ + 0, /* properties_provided */ + 0, /* properties_destroyed */ + 0, /* todo_flags_start */ + 0, /* todo_flags_finish */ +}; + +class pass_strip_predict_hints : public gimple_opt_pass +{ +public: + pass_strip_predict_hints (gcc::context *ctxt) +: gimple_opt_pass (p
Re: [PATCH] Introduce __builtin_expect_with_probability (PR target/83610).
On 08/07/2018 02:23 PM, Jan Hubicka wrote: >> 2018-07-24 Martin Liska >> >> PR target/83610 >> * builtin-types.def (BT_FN_LONG_LONG_LONG_DOUBLE): New type. >> * builtins.c (expand_builtin_expect_with_probability): >> New function. >> (expand_builtin): Handle also BUILT_IN_EXPECT_WITH_PROBABILITY. >> (build_builtin_expect_predicate): Likewise. >> (fold_builtin_expect): Likewise. >> (fold_builtin_2): Likewise. >> (fold_builtin_3): Likewise. >> * builtins.def (BUILT_IN_EXPECT_WITH_PROBABILITY): Define new >> builtin. >> * builtins.h (fold_builtin_expect): Add new argument >> (probability). >> * doc/extend.texi: Document the new builtin. >> * doc/invoke.texi: Likewise. >> * gimple-fold.c (gimple_fold_call): Pass new argument. >> * ipa-fnsummary.c (find_foldable_builtin_expect): >> Handle also BUILT_IN_EXPECT_WITH_PROBABILITY. >> * predict.c (expr_expected_value): Add new out argument which >> is probability. >> (expr_expected_value_1): Likewise. >> (tree_predict_by_opcode): Predict edge based on >> provided probability. >> (pass_strip_predict_hints::execute): Use newly added >> DECL_BUILT_IN_P macro. >> * predict.def (PRED_BUILTIN_EXPECT_WITH_PROBABILITY): >> Define new predictor. >> * tree.h (DECL_BUILT_IN_P): Define. >> >> gcc/testsuite/ChangeLog: >> >> 2018-07-24 Martin Liska >> >> * gcc.dg/predict-16.c: New test. >> * gcc.dg/predict-17.c: New test. > > OK >> +@deftypefn {Built-in Function} long __builtin_expect_with_probability >> +(long @var{exp}, long @var{c}, long @var{probability}) >> + >> +The built-in has same semantics as @code{__builtin_expect_with_probability}, >> +but user can provide expected probability (in percent) for value of >> @var{exp}. >> +Last argument @var{probability} is of flaot type and valid values > flaot->float :) Fixed. >> @@ -2426,10 +2444,10 @@ expr_expected_value_1 (tree type, tree op0, enum >> tree_code code, >> { >>tree res; >>enum br_predictor predictor2; >> - op0 = expr_expected_value (op0, visited, predictor); >> + op0 = expr_expected_value (op0, visited, predictor, probability); >>if (!op0) >> return NULL; >> - op1 = expr_expected_value (op1, visited, &predictor2); >> + op1 = expr_expected_value (op1, visited, &predictor2, probability); >>if (predictor && *predictor < predictor2) >> *predictor = predictor2; >>if (!op1) > > Here you need to combine probabilities together. > > I would probably convert it early into profile_probability type (at the time > builtin is handled) because then combination is better defined. > > If probability is known only over one path I guess you will need to turn > predictor into probability and then do the combination. The predictor > combining > logic simply takes the weaker one (assumes that they are all first match), > perhaps > with probability one can just combine probabilties and throw away predictor > info > in this case. Done that, I believe the code is now more readable. Martin > > It would be still very nice to arrange loop code to set > loop->nb_iterations_estimate > accordingly in this case. That would be useful for loop opts as reliable > hint that > number of iterations is about that much. > > Honza > >From 5df1c861976479fcedee5d428f735645a1eee089 Mon Sep 17 00:00:00 2001 From: marxin Date: Mon, 23 Jul 2018 16:03:19 +0200 Subject: [PATCH] Introduce __builtin_expect_with_probability (PR target/83610). gcc/ChangeLog: 2018-08-09 Martin Liska PR target/83610 * builtin-types.def (BT_FN_LONG_LONG_LONG_DOUBLE): Add new function type. * builtins.c (expand_builtin_expect_with_probability): New function. (expand_builtin_expect_with_probability): New function. (build_builtin_expect_predicate): Add new argumnet probability for BUILT_IN_EXPECT_WITH_PROBABILITY. (fold_builtin_expect): (fold_builtin_2): (fold_builtin_3): * builtins.def (BUILT_IN_EXPECT_WITH_PROBABILITY): * builtins.h (fold_builtin_expect): Set new argument. * doc/extend.texi: Document __builtin_expect_with_probability. * doc/invoke.texi: Likewise. * gimple-fold.c (gimple_fold_call): Pass new argument. * ipa-fnsummary.c (find_foldable_builtin_expect): Handle also BUILT_IN_EXPECT_WITH_PROBABILITY. * predict.c (get_predictor_value): New function. (expr_expected_value): Add new argument probability. Assume that predictor and probability are always non-null. (expr_expected_value_1): Likewise. For __builtin_expect and __builtin_expect_with_probability set probability. Handle combination in binary expressions. (tree_predict_by_opcode): Simplify code by simply calling get_predictor_value. (pass_strip_predict_hints::execute): Add handling of BUILT_IN_EXPECT_WITH_PROBABILITY. * predict.def (PRED_BUILTIN
[PATCH][OBVIOUS] Remove extra line in common.opt (PR c/86895).
Hi. One obvious one, error was introduced by my commit r245788. Martin gcc/ChangeLog: 2018-08-09 Martin Liska PR c/86895 * common.opt: Remove extra line. --- gcc/common.opt | 1 - 1 file changed, 1 deletion(-) diff --git a/gcc/common.opt b/gcc/common.opt index 5bb645291cf..8043cf06c21 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2765,7 +2765,6 @@ Enable basic block vectorization (SLP) on trees. fvect-cost-model= Common Joined RejectNegative Enum(vect_cost_model) Var(flag_vect_cost_model) Init(VECT_COST_MODEL_DEFAULT) Optimization -Specifies the cost model for vectorization. -fvect-cost-model=[unlimited|dynamic|cheap] Specifies the cost model for vectorization. fsimd-cost-model=
Re: [PATCH] convert braced initializers to strings (PR 71625)
On Thu, 9 Aug 2018, Martin Sebor wrote: > But for a declaration at file scope and without an initializer, > GCC warns that the array is assumed to have one element, but > then gives an error when sizeof is applied to it: That's how tentative definitions (C17 6.9.2) work. There's an implicit initializer of { 0 }, but only at the end of the translation unit, so the type is incomplete until then and sizeof cannot be applied to it. > even though the array does have a size of 1. (I guess that's > because of the [] syntax but the warning sure makes the sizeof > error surprising.) Auto declarations of arrays with no bound > and with no intializer are rejected with an error: > > char a[]; // error: array size missing in ‘a’ Tentative definitions only exist at file scope (and in standard C they can't have an incomplete type if they have internal linkage). See 6.7#7, "If an identifier for an object is declared with no linkage, the type for the object shall be complete by the end of its declarator, or by the end of its init-declarator if it has an initializer; in the case of function parameters (including in prototypes), it is the adjusted type (see 6.7.6.3) that is required to be complete." -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib
Hi Jonathan, > Committed to trunk, gcc-8-branch and gcc-7-branch. > > Rainer, please let me know if this still works on Solaris 10, thanks. I ran i386-pc-solaris2.1[01] and sparc-sun-solaris2.1[01] bootstraps last night: no regressions. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: m68k: handle more cases of TLS symbols with offset
Andreas Schwab wrote: > -void > -m68k_final_prescan_insn (rtx_insn *insn ATTRIBUTE_UNUSED, > - rtx *operands, int n_operands) > +static void > +m68k_adjust_decorated_operand (rtx op) FWIW, the prototype for m68k_final_prescan_insn in m68k-protos.h was not removed. Gunther
[C++ Patch] Tweak check_previous_goto_1 to emit hard errors instead of permerrors in some cases
Hi, over the years we reworked and improved the code in decl.c checking gotos quite a bit. Lately, in some specific unsafe cases, identify_goto issues upfront an error instead of a permerror, whereas it used to always issue a permerror. Over the last weeks a few colleagues of mine noticed that we don't do that, escalating a permerror to a plain error, in a case which is certainly unsafe - decl_jump_unsafe returns 2 - thus, if the user passes -fpermissive we end up emitting assembly completely missing labels. The straightforward patchlet below passes testing on x86_64-linux. Thanks, Paolo. / /cp 2018-08-09 Paolo Carlini * decl.c (check_previous_goto_1): When decl_jump_unsafe returns 2 emit an error instead of a permerror. /testsuite 2018-08-09 Paolo Carlini * g++.dg/init/goto3.C: Adjust for error intead of permerror. Index: cp/decl.c === --- cp/decl.c (revision 263443) +++ cp/decl.c (working copy) @@ -3191,7 +3191,8 @@ check_previous_goto_1 (tree decl, cp_binding_level if (!identified) { complained = identify_goto (decl, input_location, locus, - DK_PERMERROR); + problem > 1 + ? DK_ERROR : DK_PERMERROR); identified = 1; } if (complained) Index: testsuite/g++.dg/init/goto3.C === --- testsuite/g++.dg/init/goto3.C (revision 263443) +++ testsuite/g++.dg/init/goto3.C (working copy) @@ -15,11 +15,11 @@ adapt_parameters_next_iteration(void) case VAR_NONE: break; case VAR_DELTA: -int trunc_n_ants = 0; +int trunc_n_ants = 0; // { dg-message "initialization" } n_ants += trunc_n_ants; break; -case VAR_SWITCH: +case VAR_SWITCH: // { dg-error "jump" } break; - default: break; + default: break; // { dg-error "jump" } } }
Re: [PATCH] Strip only selected predictors after early tree passes (PR tree-optimization/85799).
> Hi. > > This patch adds an argument to the strip predictor pass. Early > version strips only selected hints that are not reliable when > inlining happens. The rest is striped in late tree passes. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Martin > > gcc/ChangeLog: > > 2018-08-09 Martin Liska > > PR tree-optimization/85799 > * passes.def: Add new pass_strip_predict_hints_early. > * predict.c (make_pass_profile): Add > pass_data_strip_predict_hints_early and > new pass pass_strip_predict_hints_early. > (strip_predictor_early): New function. > (pass_strip_predict_hints::execute): Call strip_predict_hints. > (strip_predict_hints): New. > (class pass_strip_predict_hints_early): New. > (make_pass_strip_predict_hints_early): Likewise. > * predict.def: Fix documentation. > * tree-pass.h (make_pass_strip_predict_hints_early): New. > > gcc/testsuite/ChangeLog: > > 2018-08-09 Martin Liska OK > NEXT_PASS (pass_split_functions); > - NEXT_PASS (pass_strip_predict_hints); > + NEXT_PASS (pass_strip_predict_hints_early); Perhaps we want to add bool parameter NEXT_PASS (pass_strip_predict_hints, true /* early_p */); > +/* Return true when PRED predictor should be removed after early > + tree passes. */ Please add bit of reasoning why some stuff is left for late (i.e. we want it to survive inlining so we don't get into this trap again) OK with this change. Honza
Re: [PATCH] Introduce __builtin_expect_with_probability (PR target/83610).
> gcc/ChangeLog: > > 2018-08-09 Martin Liska > > PR target/83610 > * builtin-types.def (BT_FN_LONG_LONG_LONG_DOUBLE): Add new > function type. > * builtins.c (expand_builtin_expect_with_probability): > New function. > (expand_builtin_expect_with_probability): New function. > (build_builtin_expect_predicate): Add new argumnet probability > for BUILT_IN_EXPECT_WITH_PROBABILITY. > (fold_builtin_expect): > (fold_builtin_2): > (fold_builtin_3): > * builtins.def (BUILT_IN_EXPECT_WITH_PROBABILITY): > * builtins.h (fold_builtin_expect): Set new argument. > * doc/extend.texi: Document __builtin_expect_with_probability. > * doc/invoke.texi: Likewise. > * gimple-fold.c (gimple_fold_call): Pass new argument. > * ipa-fnsummary.c (find_foldable_builtin_expect): Handle > also BUILT_IN_EXPECT_WITH_PROBABILITY. > * predict.c (get_predictor_value): New function. > (expr_expected_value): Add new argument probability. Assume > that predictor and probability are always non-null. > (expr_expected_value_1): Likewise. For __builtin_expect and > __builtin_expect_with_probability set probability. Handle > combination in binary expressions. > (tree_predict_by_opcode): Simplify code by simply calling > get_predictor_value. > (pass_strip_predict_hints::execute): Add handling of > BUILT_IN_EXPECT_WITH_PROBABILITY. > * predict.def (PRED_BUILTIN_EXPECT_WITH_PROBABILITY): Add > new predictor. > * tree.h (DECL_BUILT_IN_P): New function. > > gcc/testsuite/ChangeLog: > > 2018-08-09 Martin Liska > > PR target/83610 > * gcc.dg/predict-17.c: New test. > * gcc.dg/predict-18.c: New test. > * gcc.dg/predict-19.c: New test. > --- > gcc/builtin-types.def | 2 + > gcc/builtins.c| 65 ++--- > gcc/builtins.def | 1 + > gcc/builtins.h| 2 +- > gcc/doc/extend.texi | 9 ++ > gcc/doc/invoke.texi | 3 + > gcc/gimple-fold.c | 3 +- > gcc/ipa-fnsummary.c | 1 + > gcc/predict.c | 154 -- > gcc/predict.def | 5 + > gcc/testsuite/gcc.dg/predict-17.c | 13 +++ > gcc/testsuite/gcc.dg/predict-18.c | 31 ++ > gcc/testsuite/gcc.dg/predict-19.c | 13 +++ > gcc/tree.h| 6 ++ > 14 files changed, 244 insertions(+), 64 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/predict-17.c > create mode 100644 gcc/testsuite/gcc.dg/predict-18.c > create mode 100644 gcc/testsuite/gcc.dg/predict-19.c > > + if (TREE_CODE (res) == INTEGER_CST > + && TREE_CODE (op0) == INTEGER_CST > + && TREE_CODE (op1) == INTEGER_CST) > + { > + /* Combine binary predictions. */ > + if (*probability != -1 || probability2 != -1) > + { > + HOST_WIDE_INT p1 = get_predictor_value (*predictor, *probability); > + HOST_WIDE_INT p2 = get_predictor_value (predictor2, probability2); > + *probability = p1 * p2 / REG_BR_PROB_BASE; I guess you want to use RDIV here. Eventually we want to move to profile_probability type everywhere but I guess that can be done later. OK, thanks! Honza
Re: [PATCH] [Ada] Make middle-end string literals NUL terminated
Hi Bernd, > On 07 Aug 2018, at 19:10, Bernd Edlinger wrote: > >>> procedure Array9 is >>> I see that "1234" is put in the merge section, >>> but V1 is not. Maybe because of the alignment requirement? >>> >>> But it sould not be much different from the C test case, >>> which is now able to merge the non-zero terminated strings. >> >> I'm happy to have a look. An effect of alignments and inlining I think. I see expected behavior with calls to an external program instead of the local one. I tried the testcases we have here plus a couple of variations, e.g. with strings explicitly terminated by multiple ascii.nul, and they all seem to be working fine :-) Regarding Ada testcases for the dejagnu suite, I'm not quite sure how to go about them. For starters at least, we could for example contemplate something like: -- { dg-do compile } -- { dg-options "-O1 -fmerge-all-constants" } procedure String_Merge1 is procedure Process (X : String); pragma Import (Ada, Process); begin Process ("1234"); Process ("ABCD" & Ascii.NUL); end; -- We expect something like: -- .section .rodata.str1.1,"aMS",@progbits,1 -- .LC2: -- .string "1234" -- .LC3: -- .string "ABCD" -- Not clear how to match that generally enough. -- { dg-final { scan-assembler ".rodata.str1.1,\"aMS\"\[^\n\r\]*\n\.LC.:\n\[^\n\ \r\]*\.string \"1234\"\n\.LC.:\n\[^\n\r\]*\.string \"ABCD\"\n" } } As the comment suggests, I'm not clear if that's robust/general enough. I suppose we probably can't always expect .string, for instance. I suppose we could also have other bits between the .section and the literals. Would be fine as long as it's not something like ".section .rodata". Olivier
aarch64 - PR target/86887 Fix missing register constraints in carryin patterns
Some of the carryin insn patterns are missing a register constraint. That means that the register allocator can pick practically anything to hold that value, including memory locations, or registers of the wrong class. PR target/86887 * config/aarch64/aarch64.md (add3_carryinC_zero): Add missing register constraint to operand 0. (add3_carryinC): Likewise. (add3_carryinV_zero, add3_carryinV): Likewise. R. diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 955bf18..a73ecc7 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -2429,7 +2429,7 @@ (define_insn "*add3_carryinC_zero" (plus:GPI (match_operand:GPI 3 "aarch64_carry_operation" "") (match_dup 1) - (set (match_operand:GPI 0 "register_operand") + (set (match_operand:GPI 0 "register_operand" "=r") (plus:GPI (match_dup 3) (match_dup 1)))] "" "adcs\\t%0, %1, zr" @@ -2450,7 +2450,7 @@ (define_insn "*add3_carryinC" (match_operand:GPI 4 "aarch64_carry_operation" "") (match_dup 1)) (match_dup 2) - (set (match_operand:GPI 0 "register_operand") + (set (match_operand:GPI 0 "register_operand" "=r") (plus:GPI (plus:GPI (match_dup 4) (match_dup 1)) (match_dup 2)))] @@ -2495,7 +2495,7 @@ (define_insn "*add3_carryinV_zero" (plus:GPI (match_operand:GPI 3 "aarch64_carry_operation" "") (match_dup 1) - (set (match_operand:GPI 0 "register_operand") + (set (match_operand:GPI 0 "register_operand" "=r") (plus:GPI (match_dup 3) (match_dup 1)))] "" "adcs\\t%0, %1, zr" @@ -2516,7 +2516,7 @@ (define_insn "*add3_carryinV" (match_operand:GPI 4 "aarch64_carry_operation" "") (match_dup 1)) (match_dup 2) - (set (match_operand:GPI 0 "register_operand") + (set (match_operand:GPI 0 "register_operand" "=r") (plus:GPI (plus:GPI (match_dup 4) (match_dup 1)) (match_dup 2)))]
Re: [PATCH, RFC, rs6000] enable GIMPLE folding of vec_splat
> On Aug 7, 2018, at 2:25 PM, Will Schmidt wrote: > > Hi > Enable GIMPLE folding of the vec_splat() intrinsic. > > For review.. feedback is expected. :-) > > I came up with the following after spending some time poking around > at the tree_vec_extract() and vector_element() functions as seen > in tree-vect-generic.c looking for insights. Some of this seems a > bit clunky to me yet, but this is functional as far as make-check > can tell, and is as far as I can get without additional input. > > This uses the tree_vec_extract() function out of tree-vect-generic.c > to retrieve the splat value, which is a BIT_FIELD_REF. That function is > made non-static as part of this change. > > In review of the .gimple output, this folding takes a sample testcase > of > vector bool int testb_0 (vector bool int x) > { > return vec_splat (x, 0b0); > } > from: > testb_0 (__vector __bool int x) > { > __vector __bool intD.1486 D.2855; > > _1 = VIEW_CONVERT_EXPR<__vector signed intD.1468>(xD.2778); > _2 = __builtin_altivec_vspltwD.1919 (_1, 0); > D.2855 = VIEW_CONVERT_EXPR<__vector __bool intD.1486>(_2); > return D.2855; > } > to: > testb_0 (__vector __bool int x) > { > __vector __bool intD.1486 D.2855; > > _1 = VIEW_CONVERT_EXPR<__vector signed intD.1468>(xD.2778); > D.2856 = BIT_FIELD_REF <_1, 32, 0>; > _2 = {D.2856, D.2856, D.2856, D.2856}; > D.2855 = VIEW_CONVERT_EXPR<__vector __bool intD.1486>(_2); > return D.2855; > } > This looks okay. > > Testcases are being posted as a separate patch. > > OK for trunk? > Thanks, > -Will > > [gcc] > > 2018-08-07 Will Schmidt > > * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add support for > early gimple folding of vec_splat(). > * tree-vect-generic.c: Remove static from tree_vec_extract() definition. > * gimple-fold.h: Add an extern define for tree_vec_extract(). > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 35c32be..acc6b49 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -15764,10 +15764,56 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator > *gsi) >tree splat_tree = build_vector_from_val (TREE_TYPE (lhs), splat_value); >g = gimple_build_assign (lhs, splat_tree); >gimple_set_location (g, gimple_location (stmt)); >gsi_replace (gsi, g, true); >return true; > + } > + > +/* Flavors of vec_splat. */ > +// a = vec_splat (b, 0x3) ; // a = { b[3],b[3],b[3],...}; > +case ALTIVEC_BUILTIN_VSPLTB: > +case ALTIVEC_BUILTIN_VSPLTH: > +case ALTIVEC_BUILTIN_VSPLTW: > +case VSX_BUILTIN_XXSPLTD_V2DI: > +case VSX_BUILTIN_XXSPLTD_V2DF: > + { > + arg0 = gimple_call_arg (stmt, 0); /* input vector. */ > + arg1 = gimple_call_arg (stmt, 1); /* index into arg0. */ > + /* Only fold the vec_splat_*() if arg1 is a constant > +5-bit unsigned literal. */ > + if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 0x1f) > + return false; > + > + lhs = gimple_call_lhs (stmt); > + tree lhs_type = TREE_TYPE (lhs); > + > + tree splat; > + if (TREE_CODE (arg0) == VECTOR_CST) > + splat = VECTOR_CST_ELT (arg0, TREE_INT_CST_LOW (arg1)); You should force arg1 into range before this. It should be adjusted modulo VECTOR_CST_NELTS (arg0). Yes, the underlying vsplt* instructions will handle the modulo itself, but when expanding early we should make the correction early, else something is likely to explode. Do you have a test where arg1 is less than 32 but greater than the number of elements? > + else > + { > + /* Determine (in bits) the length and start location of the > +splat value for a call to the tree_vec_extract helper. */ > + int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (lhs_type)) > + * BITS_PER_UNIT; I expect you should use arg0's type rather than lhs_type here. They should be the same, of course; it's just clearer. > + int splat_elem_size = tree_size_in_bits / VECTOR_CST_NELTS (arg0); > + int splat_start_bit = TREE_INT_CST_LOW (arg1) * splat_elem_size; > + /* Do not attempt to early-fold if the size + specified offset into > +the vector would touch outside of the source vector. */ > + if ((splat_start_bit + splat_elem_size) > tree_size_in_bits) > + return false; This won't be necessary once you fix the modulo issue. The rest LGTM. Thanks, Bill > + tree len = build_int_cst (bitsizetype, splat_elem_size); > + tree start = build_int_cst (bitsizetype, splat_start_bit); > + splat = tree_vec_extract (gsi, TREE_TYPE (lhs_type), arg0, > + len, start); > + } > + /* And finally, build the new vector. */ > + tree splat_tree = build_vector_from_val (lhs_type, splat); > + g = gimple_build_assign (lhs
Re: [PATCH] Handle overlength strings in the C FE
On 08/01/18 22:43, Joseph Myers wrote: > On Wed, 1 Aug 2018, Marek Polacek wrote: > >> I guess you want XALLOCAVAR or XNEWVAR. > > Not XALLOCAVAR; GCC supports string constants up to 2 GB (minus one byte), > which is far too much to put on the stack. > I assume you want me to use XNEWVEC/XDELETEVEC. Attached is a new version using those macros. Is it OK for trunk? Thanks Bernd. 2018-08-01 Bernd Edlinger * c-typeck.c (digest_init): Shorten overlength strings. Index: gcc/c/c-typeck.c === --- gcc/c/c-typeck.c (revision 263072) +++ gcc/c/c-typeck.c (working copy) @@ -7435,19 +7435,17 @@ digest_init (location_t init_loc, tree type, tree } } - TREE_TYPE (inside_init) = type; if (TYPE_DOMAIN (type) != NULL_TREE && TYPE_SIZE (type) != NULL_TREE && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST) { unsigned HOST_WIDE_INT len = TREE_STRING_LENGTH (inside_init); + unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT; /* Subtract the size of a single (possibly wide) character because it's ok to ignore the terminating null char that is counted in the length of the constant. */ - if (compare_tree_int (TYPE_SIZE_UNIT (type), -(len - (TYPE_PRECISION (typ1) - / BITS_PER_UNIT))) < 0) + if (compare_tree_int (TYPE_SIZE_UNIT (type), len - unit) < 0) pedwarn_init (init_loc, 0, ("initializer-string for array of chars " "is too long")); @@ -7456,8 +7454,21 @@ digest_init (location_t init_loc, tree type, tree warning_at (init_loc, OPT_Wc___compat, ("initializer-string for array chars " "is too long for C++")); + if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0) + { + unsigned HOST_WIDE_INT size + = tree_to_uhwi (TYPE_SIZE_UNIT (type)); + const char *p = TREE_STRING_POINTER (inside_init); + char *q = XNEWVEC (char, size + unit); + + memcpy (q, p, size); + memset (q + size, 0, unit); + inside_init = build_string (size + unit, q); + XDELETEVEC (q); + } } + TREE_TYPE (inside_init) = type; return inside_init; } else if (INTEGRAL_TYPE_P (typ1))
Fix invalid assumption in vect_transform_stmt (PR 86871)
The handling of outer-loop uses of inner-loop definitions assumed that anything that wasn't a PHI would be a gassign, but as the test shows, it's also possible for it to be a gcall. Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and x86_64-linux-gnu. Applied as obvious to trunk, but OK for GCC 8 too? Richard 2018-08-09 Richard Sandiford gcc/ PR tree-optimization/86871 * tree-vect-stmts.c (vect_transform_stmt): Use gimple_get_lhs instead of gimple_assign_lhs. gcc/testsuite/ PR tree-optimization/86871 * gcc.dg/vect/pr86871.c: New test. Index: gcc/tree-vect-stmts.c === --- gcc/tree-vect-stmts.c 2018-08-01 16:29:32.291384402 +0100 +++ gcc/tree-vect-stmts.c 2018-08-09 15:34:54.968197176 +0100 @@ -9804,7 +9804,7 @@ vect_transform_stmt (stmt_vec_info stmt_ if (gimple_code (stmt) == GIMPLE_PHI) scalar_dest = PHI_RESULT (stmt); else -scalar_dest = gimple_assign_lhs (stmt); +scalar_dest = gimple_get_lhs (stmt); FOR_EACH_IMM_USE_FAST (use_p, imm_iter, scalar_dest) if (!flow_bb_inside_loop_p (innerloop, gimple_bb (USE_STMT (use_p Index: gcc/testsuite/gcc.dg/vect/pr86871.c === --- /dev/null 2018-07-26 10:26:13.137955424 +0100 +++ gcc/testsuite/gcc.dg/vect/pr86871.c 2018-08-09 15:34:54.964197212 +0100 @@ -0,0 +1,15 @@ +/* { dg-do compile } */ + +extern int b[]; +extern int c[]; +void g(int f) { + for (; f; f++) { +int d = 0; +for (int e = -1; e <= 1; e++) { + int a = f + e; + if (a) +d = *(c + a); +} +*(b + f) = d; + } + }
Allow inner-loop reductions with variable-length vectors
While working on PR 86871, I noticed we were being overly restrictive when handling variable-length vectors. For: for (i : ...) { res = ...; for (j : ...) res op= ...; a[i] = res; } we don't need a reduction operation (although we do for double reductions like: res = ...; for (i : ...) for (j : ...) res op= ...; a[i] = res; which must still be rejected). Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and x86_64-linux-gnu. OK to install? Richard 2018-08-09 Richard Sandiford gcc/ * tree-vect-loop.c (vectorizable_reduction): Allow inner-loop reductions for variable-length vectors. gcc/testsuite/ * gcc.target/aarch64/sve/reduc_8.c: New test. Index: gcc/tree-vect-loop.c === --- gcc/tree-vect-loop.c2018-08-01 16:14:50.227052736 +0100 +++ gcc/tree-vect-loop.c2018-08-09 15:38:35.230258362 +0100 @@ -6711,6 +6711,7 @@ vectorizable_reduction (stmt_vec_info st } if (reduction_type != EXTRACT_LAST_REDUCTION + && (!nested_cycle || double_reduc) && reduc_fn == IFN_LAST && !nunits_out.is_constant ()) { Index: gcc/testsuite/gcc.target/aarch64/sve/reduc_8.c === --- /dev/null 2018-07-26 10:26:13.137955424 +0100 +++ gcc/testsuite/gcc.target/aarch64/sve/reduc_8.c 2018-08-09 15:38:35.230258362 +0100 @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -ftree-vectorize" } */ + +int +reduc (int *restrict a, int *restrict b, int *restrict c) +{ + for (int i = 0; i < 100; ++i) +{ + int res = 0; + for (int j = 0; j < 100; ++j) + if (b[i + j] != 0) + res = c[i + j]; + a[i] = res; +} +} + +/* { dg-final { scan-assembler-times {\tcmpne\tp[0-9]+\.s, } 1 } } */ +/* We ought to use the CMPNE result for the SEL too. */ +/* { dg-final { scan-assembler-not {\tcmpeq\tp[0-9]+\.s, } { xfail *-*-* } } } */ +/* { dg-final { scan-assembler-times {\tsel\tz[0-9]+\.s, } 1 } } */
Restore flow_bb_inside_loop_p tests (PR 86858)
The series to remove vinfo_for_stmt also removed tests of flow_bb_inside_loop_p if the call was simply testing whether the statement was in the vectorisation region. I'd tried to keep calls that were testing whether the statement was in a particular loop (inner or outer), but messed up in vect_is_simple_reduction and removed calls that were actually needed. This patch restores them. I double-checked the other removed calls and I think these are the only ones affected. Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and x86_64-linux-gnu. Applied as obvious, on the basis that it's simply reverting part of my own patch. Richard 2018-08-08 Richard Sandiford gcc/ PR tree-optimization/86858 * tree-vect-loop.c (vect_is_simple_reduction): Restore flow_bb_inside_loop_p calls. gcc/testsuite/ PR tree-optimization/86858 * gcc.dg/vect/pr86858.c: New test.
[PATCH] i386: do not use SImode mul-highpart on 64-bit
Hello, on x86-64, 32-bit division by constants uses mulsi3_highpart pattern that turns into 'mull ' instruction with source implicitly in eax and result in edx:eax. However, using 64-bit multiplication with zero-extended source would be preferable, as the imulq instruction accepts the magic multiplier as immediate if it fits into 31 bits, has fewer register allocation constraints, and typically has better latency/throughput. Perhaps ideally we'd want expand_divmod to automatically choose this cheaper variant on x86, but changing that appears to be too complicated. On the other hand, we don't use mul_highpart patterns for anything else, so we can simply expose only the DImode mul_highpart on x86-64. This patch does that by changing mode iterator so we offer only SImode mul-highpart on 32-bit x86 and only DImode on 64-bit. Bootstrapped/regtested on x86-64, OK for trunk? Alexander PR target/82418 * config/i386/i386.md (mul3_highpart): Use DWIH mode iterator instead of SWI48. testsuite/ * gcc.target/i386/pr82418.c: New test. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 73948c12618..10783d305d2 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -7792,16 +7792,16 @@ (define_insn "*mulqihi3_1" (set_attr "mode" "QI")]) (define_expand "mul3_highpart" - [(parallel [(set (match_operand:SWI48 0 "register_operand") - (truncate:SWI48 + [(parallel [(set (match_operand:DWIH 0 "register_operand") + (truncate:DWIH (lshiftrt: (mult: (any_extend: - (match_operand:SWI48 1 "nonimmediate_operand")) + (match_operand:DWIH 1 "nonimmediate_operand")) (any_extend: - (match_operand:SWI48 2 "register_operand"))) + (match_operand:DWIH 2 "register_operand"))) (match_dup 3 - (clobber (match_scratch:SWI48 4)) + (clobber (match_scratch:DWIH 4)) (clobber (reg:CC FLAGS_REG))])] "" "operands[3] = GEN_INT (GET_MODE_BITSIZE (mode));") diff --git a/gcc/testsuite/gcc.target/i386/pr82418.c b/gcc/testsuite/gcc.target/i386/pr82418.c index e69de29bb2d..95a506d5ccd 100644 --- a/gcc/testsuite/gcc.target/i386/pr82418.c +++ b/gcc/testsuite/gcc.target/i386/pr82418.c @@ -0,0 +1,10 @@ +/* PR target/82418 */ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-options "-O2" } */ +/* { dg-final { scan-assembler "imul\[^\n\r]*1374389535" } } */ + +unsigned +f1(unsigned x) +{ + return x / 100; +}
libgo: fix spurious test failure in libgo/runtime/pprof
TestMemoryProfiler uses a too restrictive pattern that fails to match backtraces that contain two tabs after the function address. That can happen when the formatted addresses are of different length: 0: 0 [1: 2097152] @ 0x1746c 0x134a00 0x13450e 0x134fd8 0x84396 0x84892 0x17526 0x17628 0xf863a 0xbe3d8 # 0x1746b pprof.allocateReflectTransient+0x21 /suse/schwab/src/gcc/n-riscv64/riscv64-suse-linux/libgo/gotest32110/test/mprof_test.go:48 # 0x1349ffffi_call_asm+0x51 ../../../gcc/libffi/src/riscv/sysv.S:124 # 0x13450dffi_call_int+0xf7 ../../../gcc/libffi/src/riscv/ffi.c:372 # 0x134fd7reflect.call+0xab ../../../gcc/libgo/runtime/go-reflect-call.c:232 # 0x84395 reflect.Value.call+0x58d ../../../gcc/libgo/go/reflect/value.go:458 # 0x84891 reflect.Value.Call+0x4b ../../../gcc/libgo/go/reflect/value.go:306 # 0x17525 pprof.allocateReflect+0x4b /suse/schwab/src/gcc/n-riscv64/riscv64-suse-linux/libgo/gotest32110/test/mprof_test.go:53 # 0x17627 runtime_pprof.TestMemoryProfiler+0xf7 /suse/schwab/src/gcc/n-riscv64/riscv64-suse-linux/libgo/gotest32110/test/mprof_test.go:75 # 0xf8639 testing.tRunner+0x89 ../../../gcc/libgo/go/testing/testing.go:777 Andreas. diff --git a/libgo/go/runtime/pprof/mprof_test.go b/libgo/go/runtime/pprof/mprof_test.go index 5d77a1d898..ffa0591de2 100644 --- a/libgo/go/runtime/pprof/mprof_test.go +++ b/libgo/go/runtime/pprof/mprof_test.go @@ -86,26 +86,26 @@ func TestMemoryProfiler(t *testing.T) { tests := []string{ fmt.Sprintf(`%v: %v \[%v: %v\] @ 0x[0-9,a-f x]+ -# 0x[0-9,a-f]+pprof\.allocatePersistent1K\+0x[0-9,a-f]+ .*/mprof_test\.go:40 -# 0x[0-9,a-f]+runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test\.go:74 +# 0x[0-9,a-f]++pprof\.allocatePersistent1K\+0x[0-9,a-f]+ .*/mprof_test\.go:40 +# 0x[0-9,a-f]++runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test\.go:74 `, 32*memoryProfilerRun, 1024*memoryProfilerRun, 32*memoryProfilerRun, 1024*memoryProfilerRun), fmt.Sprintf(`0: 0 \[%v: %v\] @ 0x[0-9,a-f x]+ -# 0x[0-9,a-f]+pprof\.allocateTransient1M\+0x[0-9,a-f]+ .*/mprof_test.go:21 -# 0x[0-9,a-f]+runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test.go:72 +# 0x[0-9,a-f]++pprof\.allocateTransient1M\+0x[0-9,a-f]+ .*/mprof_test.go:21 +# 0x[0-9,a-f]++runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test.go:72 `, (1<<10)*memoryProfilerRun, (1<<20)*memoryProfilerRun), // This should start with "0: 0" but gccgo's imprecise // GC means that sometimes the value is not collected. fmt.Sprintf(`(0|%v): (0|%v) \[%v: %v\] @ 0x[0-9,a-f x]+ -# 0x[0-9,a-f]+pprof\.allocateTransient2M\+0x[0-9,a-f]+ .*/mprof_test.go:27 -# 0x[0-9,a-f]+runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test.go:73 +# 0x[0-9,a-f]++pprof\.allocateTransient2M\+0x[0-9,a-f]+ .*/mprof_test.go:27 +# 0x[0-9,a-f]++runtime_pprof\.TestMemoryProfiler\+0x[0-9,a-f]+ .*/mprof_test.go:73 `, memoryProfilerRun, (2<<20)*memoryProfilerRun, memoryProfilerRun, (2<<20)*memoryProfilerRun), // This should start with "0: 0" but gccgo's imprecise // GC means that sometimes the value is not collected. fmt.Sprintf(`(0|%v): (0|%v) \[%v: %v\] @( 0x[0-9,a-f]+)+ -# 0x[0-9,a-f]+pprof\.allocateReflectTransient\+0x[0-9,a-f]+ .*/mprof_test.go:48 +# 0x[0-9,a-f]++pprof\.allocateReflectTransient\+0x[0-9,a-f]+ .*/mprof_test.go:48 `, memoryProfilerRun, (2<<20)*memoryProfilerRun, memoryProfilerRun, (2<<20)*memoryProfilerRun), } -- 2.18.0 -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
[PATCH] Clarify source of tm.texi to copy for GFDL grant
When tm.texi.in is updated in the source tree, the following message gets displayed: Verify that you have permission to grant a GFDL license for all new text in tm.texi, then copy it to /gcc/doc/tm.texi. Having been myself and some colleagues confused several time by that message as to what tm.texi to copy, I think it would be clearer to indicate the absolute path for the source as well. This patch achieves that. ChangeLog entry is as follows: *** gcc/ChangeLog *** 2018-08-09 Thomas Preud'homme * Makefile.in: Clarify which tm.texi to copy over to assert the right to grant a GFDL license for all. Testing: Built GCC with a change in tm.texi.in and copied by copy/pasting the source and destination path from the resulting message. Second build then succeeded. Is this ok for trunk? Best regards, Thomas diff --git a/gcc/Makefile.in b/gcc/Makefile.in index e7d818d174c..d8d2b885f6d 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -2504,7 +2504,7 @@ s-tm-texi: build/genhooks$(build_exeext) $(srcdir)/doc/tm.texi.in else \ echo >&2 ; \ echo Verify that you have permission to grant a GFDL license for all >&2 ; \ - echo new text in tm.texi, then copy it to $(srcdir)/doc/tm.texi. >&2 ; \ + echo new text in $(objdir)/tm.texi, then copy it to $(srcdir)/doc/tm.texi. >&2 ; \ false; \ fi -- 2.18.0
Re: [PATCH] Clarify source of tm.texi to copy for GFDL grant
On Thu, 9 Aug 2018, Thomas Preudhomme wrote: > 2018-08-09 Thomas Preud'homme > > * Makefile.in: Clarify which tm.texi to copy over to assert the > right to grant a GFDL license for all. OK. -- Joseph S. Myers jos...@codesourcery.com
[committed] diagnostics: add line numbers to source (PR other/84889)
This patch adds a left margin to the lines of source (and annotations) printed by diagnostic_show_locus, so that e.g. rather than: test.c: In function 'test': test.c:12:15: error: 'struct foo' has no member named 'm_bar'; did you mean 'bar'? return ptr->m_bar; ^ bar we print: test.c: In function 'test': test.c:12:15: error: 'struct foo' has no member named 'm_bar'; did you mean 'bar'? 12 | return ptr->m_bar; | ^ | bar Similarly, for a multiline case (in C++ this time), this: bad-binary-ops.C: In function 'int test_2()': bad-binary-ops.C:26:4: error: no match for 'operator+' (operand types are 's' and 't') return (some_function () + some_other_function ()); ^~~~ becomes: bad-binary-ops.C: In function 'int test_2()': bad-binary-ops.C:26:4: error: no match for 'operator+' (operand types are 's' and 't') 25 | return (some_function () | 26 |+ some_other_function ()); |^~~~ I believe this slightly improves the readability of the output, in that it: - distinguishes between the user's source code vs the annotation lines that we're adding (the underlinings and fix-it hints here) - shows the line numbers in another place (potentially helpful for multiline diagnostics, where the user can see the line numbers directly, rather than have to figure them out relative to the caret: in the 2nd example, note how the diagnostic is reported at line 26, but the first line printed is actually line 25) I'm not sure that this is the precise format we want to go with [1], but I think it's an improvement over the status quo, and we're in stage 1 of gcc 9, so there's plenty of time to shake out issues. I've turned it on by default; it can be disabled via -fno-diagnostics-show-line-numbers (it's also turned off in the testsuite, to avoid breaking numerous existing test cases). Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds 18 PASS results to gcc.sum. Committed to trunk as r263450. Dave [1] Some possible variants: - maybe just "LL|" rather than "LL | " - maybe ':' rather than '|' - maybe we should have some leading indentation, to better split up the diagnostics visually via the left-hand column - etc gcc/ChangeLog: PR other/84889 * common.opt (fdiagnostics-show-line-numbers): New option. * diagnostic-show-locus.c (class layout): Add fields "m_show_line_numbers_p" and "m_linenum_width"; (num_digits): New function. (test_num_digits): New function. (layout::layout): Initialize new fields. Update m_x_offset logic to handle any left margin. (layout::print_source_line): Print line number when requested. (layout::start_annotation_line): New member function. (layout::print_annotation_line): Call it. (layout::print_leading_fixits): Likewise. (layout::print_trailing_fixits): Likewise. Update calls to move_to_column for new parameter. (layout::get_x_bound_for_row): Add "add_left_margin" param and use it to potentially call start_annotation_line. (layout::show_ruler): Call start_annotation_line. (selftest::test_line_numbers_multiline_range): New selftest. (selftest::diagnostic_show_locus_c_tests): Call test_num_digits and selftest::test_line_numbers_multiline_range. * diagnostic.c (diagnostic_initialize): Initialize show_line_numbers_p. * diagnostic.h (struct diagnostic_context): Add field "show_line_numbers_p". * doc/invoke.texi (Diagnostic Message Formatting Options): Add -fno-diagnostics-show-line-numbers. * dwarf2out.c (gen_producer_string): Add OPT_fdiagnostics_show_line_numbers to the ignored options. * lto-wrapper.c (merge_and_complain): Likewise to the "pick one setting" options. (append_compiler_options): Likewise to the dropped options. (append_diag_options): Likewise to the passed-on options. * opts.c (common_handle_option): Handle the new option. * toplev.c (general_init): Set up global_dc->show_line_numbers_p. gcc/testsuite/ChangeLog: PR other/84889 * gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c: New test. * gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c: New test. * gcc.dg/plugin/plugin.exp (plugin_test_list): Add the new tests. * lib/prune.exp: Add -fno-diagnostics-show-line-numbers to TEST_ALWAYS_FLAGS. --- gcc/common.opt | 4 + gcc/diagnostic-show-locus.c| 163 - gcc/diagnostic.c | 1 + gcc/diagnostic.h | 4 + gcc/doc/invoke.texi
Re: [committed] diagnostics: add line numbers to source (PR other/84889)
On Thu, 9 Aug 2018, David Malcolm wrote: > This patch adds a left margin to the lines of source (and annotations) > printed by diagnostic_show_locus, so that e.g. rather than: To confirm: if the lines contain tabs anywhere, will the code replace them by spaces when a margin is added to ensure that all lines really do get consistently indented by the same amount as part of adding the left margin? -- Joseph S. Myers jos...@codesourcery.com
Re: Allow inner-loop reductions with variable-length vectors
On August 9, 2018 4:40:41 PM GMT+02:00, Richard Sandiford wrote: >While working on PR 86871, I noticed we were being overly restrictive >when handling variable-length vectors. For: > > for (i : ...) >{ > res = ...; > for (j : ...) >res op= ...; > a[i] = res; >} > >we don't need a reduction operation (although we do for double >reductions like: > > res = ...; > for (i : ...) >for (j : ...) > res op= ...; > a[i] = res; > >which must still be rejected). > >Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and >x86_64-linux-gnu. OK to install? OK. Richard. >Richard > > >2018-08-09 Richard Sandiford > >gcc/ > * tree-vect-loop.c (vectorizable_reduction): Allow inner-loop > reductions for variable-length vectors. > >gcc/testsuite/ > * gcc.target/aarch64/sve/reduc_8.c: New test. > >Index: gcc/tree-vect-loop.c >=== >--- gcc/tree-vect-loop.c 2018-08-01 16:14:50.227052736 +0100 >+++ gcc/tree-vect-loop.c 2018-08-09 15:38:35.230258362 +0100 >@@ -6711,6 +6711,7 @@ vectorizable_reduction (stmt_vec_info st > } > > if (reduction_type != EXTRACT_LAST_REDUCTION >+ && (!nested_cycle || double_reduc) > && reduc_fn == IFN_LAST > && !nunits_out.is_constant ()) > { >Index: gcc/testsuite/gcc.target/aarch64/sve/reduc_8.c >=== >--- /dev/null 2018-07-26 10:26:13.137955424 +0100 >+++ gcc/testsuite/gcc.target/aarch64/sve/reduc_8.c 2018-08-09 >15:38:35.230258362 +0100 >@@ -0,0 +1,20 @@ >+/* { dg-do compile } */ >+/* { dg-options "-O2 -ftree-vectorize" } */ >+ >+int >+reduc (int *restrict a, int *restrict b, int *restrict c) >+{ >+ for (int i = 0; i < 100; ++i) >+{ >+ int res = 0; >+ for (int j = 0; j < 100; ++j) >+ if (b[i + j] != 0) >+res = c[i + j]; >+ a[i] = res; >+} >+} >+ >+/* { dg-final { scan-assembler-times {\tcmpne\tp[0-9]+\.s, } 1 } } */ >+/* We ought to use the CMPNE result for the SEL too. */ >+/* { dg-final { scan-assembler-not {\tcmpeq\tp[0-9]+\.s, } { xfail >*-*-* } } } */ >+/* { dg-final { scan-assembler-times {\tsel\tz[0-9]+\.s, } 1 } } */
Re: Fix invalid assumption in vect_transform_stmt (PR 86871)
On August 9, 2018 4:37:57 PM GMT+02:00, Richard Sandiford wrote: >The handling of outer-loop uses of inner-loop definitions assumed >that anything that wasn't a PHI would be a gassign, but as the test >shows, it's also possible for it to be a gcall. > >Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and >x86_64-linux-gnu. Applied as obvious to trunk, but OK for GCC 8 too? OK. Richard. >Richard > > >2018-08-09 Richard Sandiford > >gcc/ > PR tree-optimization/86871 > * tree-vect-stmts.c (vect_transform_stmt): Use gimple_get_lhs > instead of gimple_assign_lhs. > >gcc/testsuite/ > PR tree-optimization/86871 > * gcc.dg/vect/pr86871.c: New test. > >Index: gcc/tree-vect-stmts.c >=== >--- gcc/tree-vect-stmts.c 2018-08-01 16:29:32.291384402 +0100 >+++ gcc/tree-vect-stmts.c 2018-08-09 15:34:54.968197176 +0100 >@@ -9804,7 +9804,7 @@ vect_transform_stmt (stmt_vec_info stmt_ > if (gimple_code (stmt) == GIMPLE_PHI) > scalar_dest = PHI_RESULT (stmt); > else >-scalar_dest = gimple_assign_lhs (stmt); >+scalar_dest = gimple_get_lhs (stmt); > > FOR_EACH_IMM_USE_FAST (use_p, imm_iter, scalar_dest) > if (!flow_bb_inside_loop_p (innerloop, gimple_bb (USE_STMT (use_p >Index: gcc/testsuite/gcc.dg/vect/pr86871.c >=== >--- /dev/null 2018-07-26 10:26:13.137955424 +0100 >+++ gcc/testsuite/gcc.dg/vect/pr86871.c2018-08-09 15:34:54.964197212 >+0100 >@@ -0,0 +1,15 @@ >+/* { dg-do compile } */ >+ >+extern int b[]; >+extern int c[]; >+void g(int f) { >+ for (; f; f++) { >+int d = 0; >+for (int e = -1; e <= 1; e++) { >+ int a = f + e; >+ if (a) >+d = *(c + a); >+} >+*(b + f) = d; >+ } >+ }
Re: [committed] diagnostics: add line numbers to source (PR other/84889)
On Thu, 2018-08-09 at 15:40 +, Joseph Myers wrote: > On Thu, 9 Aug 2018, David Malcolm wrote: > > > This patch adds a left margin to the lines of source (and > > annotations) > > printed by diagnostic_show_locus, so that e.g. rather than: > > To confirm: if the lines contain tabs anywhere, will the code replace > them > by spaces when a margin is added to ensure that all lines really do > get > consistently indented by the same amount as part of adding the left > margin? I hadn't thought of that; bother. (that's why we make these kinds of changes in stage 1). I'm not sure that we already handled that case properly; the previous behavior was to print a leading space before the source line. I'll investigate. Thanks Dave
Re: m68k: handle more cases of TLS symbols with offset
On Aug 09 2018, Gunther Nikl wrote: > FWIW, the prototype for m68k_final_prescan_insn in m68k-protos.h was > not removed. Thanks, fixed. Andreas. * config/m68k/m68k-protos.h (m68k_final_prescan_insn): Remove prototype. diff --git a/gcc/config/m68k/m68k-protos.h b/gcc/config/m68k/m68k-protos.h index 1f6a68f62b..f412db9b31 100644 --- a/gcc/config/m68k/m68k-protos.h +++ b/gcc/config/m68k/m68k-protos.h @@ -67,7 +67,6 @@ extern rtx m68k_function_value (const_tree, const_tree); extern int emit_move_sequence (rtx *, machine_mode, rtx); extern bool m68k_movem_pattern_p (rtx, rtx, HOST_WIDE_INT, bool); extern const char *m68k_output_movem (rtx *, rtx, HOST_WIDE_INT, bool); -extern void m68k_final_prescan_insn (rtx_insn *, rtx *, int); extern bool m68k_epilogue_uses (int); /* Functions from m68k.c used in constraints.md. */ -- 2.18.0 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [nios2, committed] Define TARGET_HAVE_SPECULATION_SAFE_VALUE
On 03/08/18 23:44, Sandra Loosemore wrote: > I've checked in this patch to fix the c-c++-common/spec-barrier-1.c test > failure on nios2. Per previous discussions about Spectre > vulnerabilities with Altera/Intel, this architecture is not affected so > no special handling is required here. > > -Sandra > > gcc.log > > > 2018-08-03 Sandra Loosemore > > gcc/ > * config/nios2/nios2.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): > Define. > I nearly missed this patch for my accumulated back-porting list since it didn't have the PR number in it. Just adding it so that I can track things properly. The original commit landed as r263301. R. > > gcc.patch > > > Index: gcc/config/nios2/nios2.c > === > --- gcc/config/nios2/nios2.c (revision 263298) > +++ gcc/config/nios2/nios2.c (working copy) > @@ -5572,6 +5572,9 @@ nios2_adjust_reg_alloc_order (void) > #undef TARGET_CONSTANT_ALIGNMENT > #define TARGET_CONSTANT_ALIGNMENT constant_alignment_word_strings > > +#undef TARGET_HAVE_SPECULATION_SAFE_VALUE > +#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed > + > struct gcc_target targetm = TARGET_INITIALIZER; > > #include "gt-nios2.h" > diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 9d5a23f..e9b764d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -370,6 +370,7 @@ 2018-08-03 Sandra Loosemore + PR target/86799 * config/nios2/nios2.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): Define.
[committed][PR middle-end/86897] Disable DOM in test compromised by DOM
This should have been committed with the most recent DOM change. DOM compromises this test now that it uses the EVRP range data to discover more constants. The fix is trivial, disable DOM for this test. Committed to the trunk. Jeff commit 08482a36034ea41b202c26589fa0907f8c25ff90 Author: law Date: Thu Aug 9 17:11:45 2018 + PR middle-end/86897 * gcc.dg/uninit-suppress_2.c: Disable DOM. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@263454 138bc75d-0d04-0410-961f-82ee72b054a4 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 8ea8530243d..1e915bc1a0a 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-08-09 Jeff Law + + PR middle-end/86897 + * gcc.dg/uninit-suppress_2.c: Disable DOM. + 2018-08-09 Richard Sandiford * gcc.target/aarch64/sve/reduc_8.c: New test. diff --git a/gcc/testsuite/gcc.dg/uninit-suppress_2.c b/gcc/testsuite/gcc.dg/uninit-suppress_2.c index af0f192f0c0..b617609f5e1 100644 --- a/gcc/testsuite/gcc.dg/uninit-suppress_2.c +++ b/gcc/testsuite/gcc.dg/uninit-suppress_2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */ +/* { dg-options "-fno-tree-dominator-opts -fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */ void blah(); void bar (int); int gflag;
Re: m68k: handle more cases of TLS symbols with offset
On 08/09/2018 10:25 AM, Andreas Schwab wrote: > On Aug 09 2018, Gunther Nikl wrote: > >> FWIW, the prototype for m68k_final_prescan_insn in m68k-protos.h was >> not removed. > > Thanks, fixed. > > Andreas. > > * config/m68k/m68k-protos.h (m68k_final_prescan_insn): Remove > prototype. > > diff --git a/gcc/config/m68k/m68k-protos.h b/gcc/config/m68k/m68k-protos.h > index 1f6a68f62b..f412db9b31 100644 > --- a/gcc/config/m68k/m68k-protos.h > +++ b/gcc/config/m68k/m68k-protos.h > @@ -67,7 +67,6 @@ extern rtx m68k_function_value (const_tree, const_tree); > extern int emit_move_sequence (rtx *, machine_mode, rtx); > extern bool m68k_movem_pattern_p (rtx, rtx, HOST_WIDE_INT, bool); > extern const char *m68k_output_movem (rtx *, rtx, HOST_WIDE_INT, bool); > -extern void m68k_final_prescan_insn (rtx_insn *, rtx *, int); > extern bool m68k_epilogue_uses (int); > > /* Functions from m68k.c used in constraints.md. */ > There's also an unused variable in m68k_adjust_decorated_operand which causes bootstrap to fail. /src/gcc/gcc/config/m68k/m68k.c: In function 'void m68k_adjust_decorated_operand(rtx)': /src/gcc/gcc/config/m68k/m68k.c:2337:7: error: unused variable 'i' [-Werror=unused-variable] int i; ^ I'm committing the attached patch as an obvious fix. The tester will spin it overnight. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index dfe1c7c8e0f..b215706ccfa 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2018-08-09 Jeff Law + + * config/m68k/m68k.c (m68k_adjust_decorated_operand): Remove + unused variable. + 2018-08-09 Andreas Schwab * config/m68k/m68k-protos.h (m68k_final_prescan_insn): Remove diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c index 303dfc1c0c9..ea4154583b1 100644 --- a/gcc/config/m68k/m68k.c +++ b/gcc/config/m68k/m68k.c @@ -2334,8 +2334,6 @@ m68k_unwrap_symbol (rtx orig, bool unwrap_reloc32_p) static void m68k_adjust_decorated_operand (rtx op) { - int i; - /* Combine and, possibly, other optimizations may do good job converting (const (unspec [(symbol)]))
Re: libgo: fix spurious test failure in libgo/runtime/pprof
On Thu, Aug 9, 2018 at 8:03 AM, Andreas Schwab wrote: > TestMemoryProfiler uses a too restrictive pattern that fails to match > backtraces that contain two tabs after the function address. That can > happen when the formatted addresses are of different length: Your patch seems to be whitespace only and to be honest I'm not sure exactly what changed. Would it be possible to send it as an attachment? Ian
Improve safe iterator move semantic
Here is a patch to improve Debug mode safe iterator move semantic. To summarize where we used to have N mutex locks we now have N - 1. For instance move constructor used to lock mutex twice, now it only does it once. Note that move constructor or move assignment operator are currently more expensive than their copy counterparts ! I did my best to avoid new symbols but I still require 1 to be added: _Safe_local_iterator::_M_attach_single. We will now make use of the already exported _Safe_iterator::_M_attach_single symbol. Tested under Linux x86_64, Debug mode. Ok to commit ? François diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 593783d..1143d9a 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2046,6 +2046,7 @@ GLIBCXX_3.4.26 { _ZNSt3pmr25monotonic_buffer_resource13_M_new_bufferE[jmy][jmy]; _ZNSt3pmr25monotonic_buffer_resource18_M_release_buffersEv; +_ZN11__gnu_debug25_Safe_local_iterator_base16_M_detach_singleEv; } GLIBCXX_3.4.25; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h index f58a78f..c276a18 100644 --- a/libstdc++-v3/include/debug/safe_base.h +++ b/libstdc++-v3/include/debug/safe_base.h @@ -97,6 +97,13 @@ namespace __gnu_debug : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0) { this->_M_attach(__x._M_sequence, __constant); } +#if __cplusplus >= 201103L +_Safe_iterator_base(_Safe_iterator_base&&) = default; + +_Safe_iterator_base& +operator=(_Safe_iterator_base&&) = default; +#endif + ~_Safe_iterator_base() { this->_M_detach(); } /** For use in _Safe_iterator. */ @@ -121,6 +128,12 @@ namespace __gnu_debug void _M_detach(); +#if __cplusplus >= 201103L +/** Attaches this iterator at __x place. */ +void +_M_move_to(_Safe_iterator_base* __x, bool __constant); +#endif + public: /** Likewise, but not thread-safe. */ void @@ -148,11 +161,12 @@ namespace __gnu_debug /** Reset all member variables */ void -_M_reset() throw (); +_M_reset() _GLIBCXX_USE_NOEXCEPT +{ __builtin_memset(this, 0, sizeof(_Safe_iterator_base)); } /** Unlink itself */ void -_M_unlink() throw () +_M_unlink() _GLIBCXX_USE_NOEXCEPT { if (_M_prior) _M_prior->_M_next = _M_next; @@ -273,6 +287,28 @@ namespace __gnu_debug void _M_detach_single(_Safe_iterator_base* __it) throw (); }; + +#if __cplusplus >= 201103L + inline void + _Safe_iterator_base::_M_move_to(_Safe_iterator_base* __x, bool __constant) + { +__gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); +if (_M_prior) + _M_prior->_M_next = this; +else + { + // No prior, we are at the beginning of the linked list. + auto& __its = __constant + ? _M_sequence->_M_const_iterators : _M_sequence->_M_iterators; + if (__its == __x) + __its = this; + } + +if (_M_next) + _M_next->_M_prior = this; + } +#endif + } // namespace __gnu_debug #endif diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index b8256fc..4b5773f 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -151,17 +151,20 @@ namespace __gnu_debug * @post __x is singular and unattached */ _Safe_iterator(_Safe_iterator&& __x) noexcept - : _Iter_base() + : _Iter_base(__x), _Safe_base(std::move(__x)) { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() || __x.base() == _Iterator(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); - _Safe_sequence_base* __seq = __x._M_sequence; - __x._M_detach(); - std::swap(base(), __x.base()); - _M_attach(__seq); + if (__x._M_sequence) + { + _M_move_to(&__x, _S_constant()); + + __x._M_reset(); + __x.base() = _Iter_base(); + } } #endif @@ -238,16 +241,22 @@ namespace __gnu_debug { __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); base() = __x.base(); - _M_version = __x._M_sequence->_M_version; + _M_version = __x._M_version; + __x._M_detach_single(); } else { _M_detach(); base() = __x.base(); - _M_attach(__x._M_sequence); + + if (__x._M_sequence) + { + _Safe_base::operator=(std::move(__x)); + _M_move_to(&__x, _S_constant()); + __x._M_reset(); + } } - __x._M_detach(); __x.base() = _Iterator(); return *this; } diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h index f9597a6..2dd9a19 100644 --- a/libstdc++-v3/include/debug/safe_local_iterator.h +++ b/libstdc++-v3/include/debug/safe_local_iterator.h @@ -112,17 +112,20 @@ namespace __gnu_debug * @post __x is singular
Re: [PATCH] assume sprintf formatting of wide characters may fail (PR 86853)
>> + bool one_2_one_ascii >> += (target_to_host_charmap[0] == 1 && target_to_host ('a') == >> 97); > Hmm. Is this really sufficient?I have nowhere near enough knowledge > of the potential target character sets to know if this is sufficiently > tight. Shouldn't it be target_to_host (97) == 'a' ? Bernd.
Re: Restore flow_bb_inside_loop_p tests (PR 86858)
Richard Sandiford writes: > The series to remove vinfo_for_stmt also removed tests of > flow_bb_inside_loop_p if the call was simply testing whether the > statement was in the vectorisation region. I'd tried to keep calls > that were testing whether the statement was in a particular loop > (inner or outer), but messed up in vect_is_simple_reduction and > removed calls that were actually needed. This patch restores them. > > I double-checked the other removed calls and I think these are > the only ones affected. > > Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and > x86_64-linux-gnu. Applied as obvious, on the basis that it's simply > reverting part of my own patch. > > Richard > > > 2018-08-08 Richard Sandiford > > gcc/ > PR tree-optimization/86858 > * tree-vect-loop.c (vect_is_simple_reduction): Restore > flow_bb_inside_loop_p calls. > > gcc/testsuite/ > PR tree-optimization/86858 > * gcc.dg/vect/pr86858.c: New test. Sorry, patch was: Index: gcc/tree-vect-loop.c === --- gcc/tree-vect-loop.c2018-08-09 15:38:35.230258362 +0100 +++ gcc/tree-vect-loop.c2018-08-09 15:41:10.672888995 +0100 @@ -2922,7 +2922,8 @@ vect_is_simple_reduction (loop_vec_info } stmt_vec_info def_stmt_info = loop_info->lookup_def (loop_arg); - if (!def_stmt_info) + if (!def_stmt_info + || !flow_bb_inside_loop_p (loop, gimple_bb (def_stmt_info->stmt))) return NULL; if (gassign *def_stmt = dyn_cast (def_stmt_info->stmt)) @@ -3161,6 +3162,7 @@ vect_is_simple_reduction (loop_vec_info && def2_info->stmt == phi && (code == COND_EXPR || !def1_info + || !flow_bb_inside_loop_p (loop, gimple_bb (def1_info->stmt)) || vect_valid_reduction_input_p (def1_info))) { if (dump_enabled_p ()) @@ -3172,6 +3174,7 @@ vect_is_simple_reduction (loop_vec_info && def1_info->stmt == phi && (code == COND_EXPR || !def2_info + || !flow_bb_inside_loop_p (loop, gimple_bb (def2_info->stmt)) || vect_valid_reduction_input_p (def2_info))) { if (! nested_in_vect_loop && orig_code != MINUS_EXPR) Index: gcc/testsuite/gcc.dg/vect/pr86858.c === --- /dev/null 2018-07-26 10:26:13.137955424 +0100 +++ gcc/testsuite/gcc.dg/vect/pr86858.c 2018-08-09 15:41:10.672888995 +0100 @@ -0,0 +1,13 @@ +/* { dg-do compile } */ + +int a, b, c, d; +char e(char f, char g) { return f + g; } +void h() { + for (; c; ++c) { +d = 0; +for (; d != 8; d = e(d, 3)) { + a = b && a; + b = c; +} + } +}
Re: [committed] diagnostics: add line numbers to source (PR other/84889)
On Thu, 2018-08-09 at 11:45 -0400, David Malcolm wrote: > On Thu, 2018-08-09 at 15:40 +, Joseph Myers wrote: > > On Thu, 9 Aug 2018, David Malcolm wrote: > > > > > This patch adds a left margin to the lines of source (and > > > annotations) > > > printed by diagnostic_show_locus, so that e.g. rather than: > > > > To confirm: if the lines contain tabs anywhere, will the code > > replace > > them > > by spaces when a margin is added to ensure that all lines really do > > get > > consistently indented by the same amount as part of adding the > > left > > margin? > > I hadn't thought of that; bother. (that's why we make these kinds of > changes in stage 1). > > I'm not sure that we already handled that case properly; the previous > behavior was to print a leading space before the source line. > > I'll investigate. It turns out that we convert tab characters to *single* space characters when printing source code. This behavior has been present since Manu first implemented -fdiagnostics-show-caret in r186305 (aka 5a9830842f69ebb059061e26f8b0699cbd85121e, PR 24985), where it was this logic (there in diagnostic.c's diagnostic_show_locus): char c = *line == '\t' ? ' ' : *line; pp_character (context->printer, c); (that logic is now in diagnostic-show-locus.c in layout::print_source_line) Arguably this is a bug, but it's intimately linked to the way in which we track "column numbers". Our "column numbers" are currently simply a 1-based byte-count, I believe, so a tab character is treated by us as simply an increment of 1 right now. There are similar issues with multibyte characters, which are being tracked in PR 49973. Adding the left margin with line numbers doesn't change this bug, but makes fixing it slightly more complicated. I've opened PR other/86904 ("Column numbers ignore tab characters") to track it. Dave
Re: [PATCH] assume sprintf formatting of wide characters may fail (PR 86853)
On 08/08/2018 10:14 PM, Jeff Law wrote: On 08/04/2018 12:46 PM, Martin Sebor wrote: The sprintf handling of wide characters neglects to consider that calling the function may fail due to a conversion error (when the wide character is invalid or not representable in the current locale). The handling also misinterprets the POSIX %S wide string directive as a plain narrow %s and doesn't include %C (the POSIX equivalent of %lc). The attached patch corrects these oversights by extending the data structures to indicate when a directive may fail, and extending the UNDER4K member of the format_result structure to also encode calls with such directives. Tested on x86_64-linux. Besides the trunk, since this bug can affect code correctness I'd like to backport this patch to both release branches (7 and 8). Martin gcc-86853.diff PR tree-optimization/86853 - sprintf optimization for wide strings doesn't account for conversion failure gcc/ChangeLog: PR tree-optimization/86853 * gimple-ssa-sprintf.c (struct format_result): Rename member. (struct fmtresult): Add member and initialize it in ctors. (format_character): Handle %C. Extend range to NUL. Set MAYFAIL. (format_string): Handle %S the same as %ls. Set MAYFAIL. (format_directive): Set POSUNDER4K when MAYFAIL is set. (parse_directive): Handle %C same as %c. (sprintf_dom_walker::compute_format_length): Adjust. (is_call_safe): Adjust. gcc/testsuite/ChangeLog: PR tree-optimization/86853 * gcc.dg/tree-ssa/builtin-sprintf-10.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-11.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Adjust. Mostly OK. One nit noted below and one question. If you're sufficiently confident that the charmap test is OK, then you just need to address the nit in the comment and you're good to go. Index: gcc/gimple-ssa-sprintf.c === --- gcc/gimple-ssa-sprintf.c(revision 263295) +++ gcc/gimple-ssa-sprintf.c(working copy) @@ -2217,13 +2224,20 @@ format_character (const directive &dir, tree arg, res.range.likely = 0; res.range.unlikely = 0; } - else if (min > 0 && min < 128) + else if (min >= 0 && min < 128) { + /* Be conservative if the target execution character set +is not a 1-to-1 mapping to the source character set or +if the source set is not ASCII. */ + bool one_2_one_ascii + = (target_to_host_charmap[0] == 1 && target_to_host ('a') == 97); Hmm. Is this really sufficient?I have nowhere near enough knowledge of the potential target character sets to know if this is sufficiently tight. I believe it is safe with all but EBCDIC character sets (in fact, the target_to_host_charmap[0] == 1 condition is unnecessary: it's implied by the target-to-host inequality). It's only used to avoid excessive noise from warnings (max), otherwise the output range for all multibyte characters is set to [1, MB_LEN_MAX] which is as permissive as it can be. I verified it with the available locales on AIX and on Linux. I don't have access to Solaris but it implements (and documents) the C99 identity requirement that char == wchar_t for all narrow chars. Windows uses UTF-16 for wchar_t so it also holds there. We can also set may_fail to true too. I didn't do it only because of the above. Martin PS I also tested a few oddball Linux charmaps with apparent gaps in the low 127 bytes. A number of them are invalid and cause GCC to ICE, e.g., DIN_66003: I track it in bug 82700 (it would be nice to handle this more gracefully). Some other invalid ones fail with a source code conversion error (e.g. LATIN-GREEK which doesn't contain all the letters of the English alphabet). I haven't found where the 1-to-1 correspondence doesn't hold @@ -2302,6 +2320,10 @@ format_string (const directive &dir, tree arg, vr_ /* Even a non-empty wide character string need not convert into any bytes. */ res.range.min = 0; + + /* A non-emtpy wide character conversion may fail. */ s/emtpy/empty/ Jeff
Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments
On Mon, Aug 06, 2018 at 12:02:31AM +1000, Jason Merrill wrote: > > OK -- see the patch below. Now, I'm not crazy about adding another bit > > to struct conversion, but reusing any of the other bits didn't seem > > safe/possible. Maybe it'll come in handy when dealing with this problem > > for function templates. > > Looks good. > > >> >> @@ -6669,9 +6669,12 @@ convert_nontype_argument (tree type, tree expr, > >> >> tsubst_flags_t complain) > >> >> /* C++17: A template-argument for a non-type > >> >> template-parameter shall > >> >> be a converted constant expression (8.20) of the type of > >> >> the > >> >> template-parameter. */ > >> >> + int errs = errorcount; > >> >> expr = build_converted_constant_expr (type, expr, complain); > >> >> if (expr == error_mark_node) > >> >> - return error_mark_node; > >> >> + /* Make sure we return NULL_TREE only if we have really > >> >> issued > >> >> + an error, as described above. */ > >> >> + return errorcount > errs ? NULL_TREE : error_mark_node; > >> > > >> > Is this still needed? > >> > >> Earlier you wrote, > >> > >> > Checking complain doesn't work because that doesn't > >> > say if we have really issued an error. If we have not, and we return > >> > NULL_TREE anyway, we hit this assert: > >> > 8517 gcc_assert (!(complain & tf_error) || seen_error ()); > >> > >> If (complain & tf_error), we shouldn't see error_mark_node without an > >> error having been issued. Why is build_converted_constant_expr > >> returning error_mark_node without giving an error when (complain & > >> tf_error)? > > > > This can happen on invalid code; e.g. tests nontype13.C and crash87.C. > > What happens there is that we're trying to convert a METHOD_TYPE to bool, > > which doesn't work: in standard_conversion we go into the > > else if (tcode == BOOLEAN_TYPE) > > block, which returns NULL, implicit_conversion also then returns NULL, yet > > no error has been issued. > > Yes, that's normal. The problem is in build_converted_constant_expr: > > if (conv) > expr = convert_like (conv, expr, complain); > else > expr = error_mark_node; > > Here when setting expr to error_mark_node I forgot to give an error if > (complain & tf_error). Done. A few testcases needed adjusting but nothing surprising. > >> >> + /* If we're in a template and we know the constant value, we can > >> >> +warn. Otherwise wait for instantiation. */ > >> >> + || (processing_template_decl && !TREE_CONSTANT (init))) > >> > > >> > I don't think we want this condition. If the value is non-constant > >> > but also not dependent, it'll never be constant, so we can go ahead > >> > and complain. > > > > (This to be investigated later.) > > Why later? So this was this wrong error: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00159.html which was confusing. But I noticed it's because we instantiate 'size' twice: in compute_array_index_type: size = instantiate_non_dependent_expr_sfinae (size, complain); size = build_converted_constant_expr (size_type_node, size, complain); size = maybe_constant_value (size); and then in check_narrowing: init = fold_non_dependent_expr (init, complain); (in this case, size is a call to constexpr user-defined conversion). But check_narrowing now returns early if instantiation_dependent_expression_p, so I thought perhaps we could simply call maybe_constant_value which fixes this problem and doesn't regress anything. Does that seem like a sensible thing to do? > > --- gcc/cp/decl.c > > +++ gcc/cp/decl.c > > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree name, tree size, > > tsubst_flags_t complain) > > { > >tree folded = cp_fully_fold (size); > >if (TREE_CODE (folded) == INTEGER_CST) > > - pedwarn (location_of (size), OPT_Wpedantic, > > + pedwarn (input_location, OPT_Wpedantic, > > It should work to use location_of (osize) here. I dropped this hunk altogether. Because location_of will use DECL_SOURCE_LOCATION for DECLs, the error message will point to the declaration itself, not the use. I don't really care either way. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-08-09 Marek Polacek PR c++/57891 * call.c (struct conversion): Add check_narrowing_const_only. (build_converted_constant_expr): Set check_narrowing and check_narrowing_const_only. Give error if expr is error node. (convert_like_real): Pass it to check_narrowing. * cp-tree.h (check_narrowing): Add a default parameter. * pt.c (convert_nontype_argument): Return NULL_TREE if tf_error. * typeck2.c (check_narrowing): Don't warn for instantiation-dependent expressions. Call maybe_constant_value instead of fold_non_dependent_expr. Don't mention { } in diagnostic. Only check narrowing for constants if CONST_ONLY.
Re: [PATCH, RFC, rs6000] enable GIMPLE folding of vec_splat
On Thu, 2018-08-09 at 08:53 -0500, Bill Schmidt wrote: > > On Aug 7, 2018, at 2:25 PM, Will Schmidt wrote: > > > > Hi > > Enable GIMPLE folding of the vec_splat() intrinsic. > > > > For review.. feedback is expected. :-) > > > > I came up with the following after spending some time poking around > > at the tree_vec_extract() and vector_element() functions as seen > > in tree-vect-generic.c looking for insights. Some of this seems a > > bit clunky to me yet, but this is functional as far as make-check > > can tell, and is as far as I can get without additional input. > > > > This uses the tree_vec_extract() function out of tree-vect-generic.c > > to retrieve the splat value, which is a BIT_FIELD_REF. That function is > > made non-static as part of this change. > > > > In review of the .gimple output, this folding takes a sample testcase > > of > > vector bool int testb_0 (vector bool int x) > > { > > return vec_splat (x, 0b0); > > } > > from: > > testb_0 (__vector __bool int x) > > { > > __vector __bool intD.1486 D.2855; > > > > _1 = VIEW_CONVERT_EXPR<__vector signed intD.1468>(xD.2778); > > _2 = __builtin_altivec_vspltwD.1919 (_1, 0); > > D.2855 = VIEW_CONVERT_EXPR<__vector __bool intD.1486>(_2); > > return D.2855; > > } > > to: > > testb_0 (__vector __bool int x) > > { > > __vector __bool intD.1486 D.2855; > > > > _1 = VIEW_CONVERT_EXPR<__vector signed intD.1468>(xD.2778); > > D.2856 = BIT_FIELD_REF <_1, 32, 0>; > > _2 = {D.2856, D.2856, D.2856, D.2856}; > > D.2855 = VIEW_CONVERT_EXPR<__vector __bool intD.1486>(_2); > > return D.2855; > > } > > > This looks okay. > > > > Testcases are being posted as a separate patch. > > > > OK for trunk? > > Thanks, > > -Will > > > > [gcc] > > > > 2018-08-07 Will Schmidt > > > > * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add support for > > early gimple folding of vec_splat(). > > * tree-vect-generic.c: Remove static from tree_vec_extract() definition. > > * gimple-fold.h: Add an extern define for tree_vec_extract(). > > > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > > index 35c32be..acc6b49 100644 > > --- a/gcc/config/rs6000/rs6000.c > > +++ b/gcc/config/rs6000/rs6000.c > > @@ -15764,10 +15764,56 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator > > *gsi) > > tree splat_tree = build_vector_from_val (TREE_TYPE (lhs), splat_value); > > g = gimple_build_assign (lhs, splat_tree); > > gimple_set_location (g, gimple_location (stmt)); > > gsi_replace (gsi, g, true); > > return true; > > + } > > + > > +/* Flavors of vec_splat. */ > > +// a = vec_splat (b, 0x3) ; // a = { b[3],b[3],b[3],...}; > > +case ALTIVEC_BUILTIN_VSPLTB: > > +case ALTIVEC_BUILTIN_VSPLTH: > > +case ALTIVEC_BUILTIN_VSPLTW: > > +case VSX_BUILTIN_XXSPLTD_V2DI: > > +case VSX_BUILTIN_XXSPLTD_V2DF: > > + { > > + arg0 = gimple_call_arg (stmt, 0); /* input vector. */ > > + arg1 = gimple_call_arg (stmt, 1); /* index into arg0. */ > > + /* Only fold the vec_splat_*() if arg1 is a constant > > + 5-bit unsigned literal. */ > > + if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 0x1f) > > + return false; > > + > > + lhs = gimple_call_lhs (stmt); > > + tree lhs_type = TREE_TYPE (lhs); > > + > > + tree splat; > > + if (TREE_CODE (arg0) == VECTOR_CST) > > + splat = VECTOR_CST_ELT (arg0, TREE_INT_CST_LOW (arg1)); > > You should force arg1 into range before this. It should be adjusted modulo > VECTOR_CST_NELTS (arg0). Yes, the underlying vsplt* instructions will OK. > handle the modulo itself, but when expanding early we should make the > correction early, else something is likely to explode. Do you have a test > where arg1 is less than 32 but greater than the number of elements? Almost. :-) I test for arg1 < 32 and > #elms, but not with the arg0-is-constant scenario. I'll craft up a couple more testcases for that. > > > + else > > + { > > + /* Determine (in bits) the length and start location of the > > + splat value for a call to the tree_vec_extract helper. */ > > + int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (lhs_type)) > > + * BITS_PER_UNIT; > > I expect you should use arg0's type rather than lhs_type here. They > should be the same, of course; it's just clearer. ok. > > > + int splat_elem_size = tree_size_in_bits / VECTOR_CST_NELTS (arg0); > > + int splat_start_bit = TREE_INT_CST_LOW (arg1) * splat_elem_size; > > + /* Do not attempt to early-fold if the size + specified offset into > > + the vector would touch outside of the source vector. */ > > + if ((splat_start_bit + splat_elem_size) > tree_size_in_bits) > > + return false; > > This won't be necessary once you fix the modulo issue. ok. > The rest LGTM. thanks for the review and feedback. :-) -Will > > T
Re: [PATCH] convert braced initializers to strings (PR 71625)
On Wed, 8 Aug 2018, Martin Sebor wrote: > Done in the attached patch. I've also avoided dealing with > zero-length arrays and added tests to make sure their size > stays is regardless of the form of their initializer and > the appropriate warnings are issued. The C front-end changes in this patch version are OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Handle overlength strings in the C FE
On Thu, 9 Aug 2018, Bernd Edlinger wrote: > On 08/01/18 22:43, Joseph Myers wrote: > > On Wed, 1 Aug 2018, Marek Polacek wrote: > > > >> I guess you want XALLOCAVAR or XNEWVAR. > > > > Not XALLOCAVAR; GCC supports string constants up to 2 GB (minus one byte), > > which is far too much to put on the stack. > > > > I assume you want me to use XNEWVEC/XDELETEVEC. > > Attached is a new version using those macros. > Is it OK for trunk? This version is OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] v2: Formatted printing for dump_* in the middle-end
On Thu, 2 Aug 2018, David Malcolm wrote: > (a) the c-format.c changes (Joseph?), and The c-format.c changes are OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch] Do not call the linker if we are creating precompiled header files
On Fri, 22 Jun 2018, Steve Ellcey wrote: > I can see both sides of this and don't feel strongly one way or the > other. I have attached a new patch that does allow for the use of just > -l options to force the linker to be called. I don't think this patch achieves the desired semantics. My understanding of the desired semantics is: just as doing plain gcc or gcc -O2 produces an error gcc: fatal error: no input files compilation terminated. so the same should apply if a linker option is passed, e.g. gcc -Wl,-rpath,/some/where (rather than the present attempt to link) but if a -l argument is passed, that should be treated as an input file and the driver should continue to try to link just as it does at present. And then, for the case you were concerned with, building precompiled headers, all three commands, with the name of a header to compile to a PCH file added to them, should again act the same: compile the header to a PCH file, do not try to link. But actually, what I see with the patch applied does not follow that. If I do ./gcc/xgcc -B./gcc/ -lm it does *not* try to link, just exits silently. And if I do ./gcc/xgcc -B./gcc/ -Wl,-nosuchoption it does *not* complain about a lack of input files, but again just exits silently. However, if I do ./gcc/xgcc -B./gcc/ -Wl,-rpath,/some/where it *does* try to link, when by the expected semantics, it should complain about the lack of input files. It appears that it's caring about whether the -Wl, arguments start with '-' or not, which is completely wrong - /some/where above is an argument to the -rpath linker option, it's not an input file. (For the next patch revision also note the "linnked" typo in a comment.) -- Joseph S. Myers jos...@codesourcery.com
[PATCH] Define aliases for containers using polymorphic_allocator
These aliases are placed in the top-level header, e.g. not . This ensures that they refer to whichever of std::vector or __debug::vector or __profile::vector is in use when the header is included. * include/std/deque (std::pmr::deque): Declare alias. * include/std/forward_list (std::pmr::forward_list): Likewise. * include/std/list (std::pmr::list): Likewise. * include/std/map (std::pmr::map, std::pmr::multimap): Likewise. * include/std/regex (std::pmr::match_results, std::pmr::cmatch) (std::pmr::smatch, std::pmr::wcmatch, std::pmr::wsmatch): Likewise. * include/std/set (std::pmr::set, std::pmr::multiset): Likewise. * include/std/string (std::pmr::basic_string, std::pmr::string) (std::pmr::u16string, std::pmr::u32string, std::pmr::wstring): Likewise. * include/std/unordered_map (std::pmr::unordered_map) (std::pmr::unordered_multimap): Likewise. * include/std/unordered_set (std::pmr::unordered_set) (std::pmr::unordered_multiset): Likewise. * include/std/vector (std::pmr::vector): Likewise. * testsuite/21_strings/basic_string/types/pmr_typedefs.cc: New test. * testsuite/23_containers/deque/types/pmr_typedefs.cc: New test. * testsuite/23_containers/forward_list/pmr_typedefs.cc: New test. * testsuite/23_containers/list/pmr_typedefs.cc: New test. * testsuite/23_containers/map/pmr_typedefs.cc: New test. * testsuite/23_containers/multimap/pmr_typedefs.cc: New test. * testsuite/23_containers/multiset/pmr_typedefs.cc: New test. * testsuite/23_containers/set/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_map/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_multimap/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_multiset/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_set/pmr_typedefs.cc: New test. * testsuite/23_containers/vector/pmr_typedefs.cc: New test. * testsuite/28_regex/match_results/pmr_typedefs.cc: New test. Tested x86_64-linux, committed to trunk. commit e7d07f1b8a628402d8a27ba382c66c461a4973a1 Author: Jonathan Wakely Date: Thu Aug 9 22:16:33 2018 +0100 Define aliases for containers using polymorphic_allocator These aliases are placed in the top-level header, e.g. not . This ensures that they refer to whichever of std::vector or __debug::vector or __profile::vector is in use when the header is included. * include/std/deque (std::pmr::deque): Declare alias. * include/std/forward_list (std::pmr::forward_list): Likewise. * include/std/list (std::pmr::list): Likewise. * include/std/map (std::pmr::map, std::pmr::multimap): Likewise. * include/std/regex (std::pmr::match_results, std::pmr::cmatch) (std::pmr::smatch, std::pmr::wcmatch, std::pmr::wsmatch): Likewise. * include/std/set (std::pmr::set, std::pmr::multiset): Likewise. * include/std/string (std::pmr::basic_string, std::pmr::string) (std::pmr::u16string, std::pmr::u32string, std::pmr::wstring): Likewise. * include/std/unordered_map (std::pmr::unordered_map) (std::pmr::unordered_multimap): Likewise. * include/std/unordered_set (std::pmr::unordered_set) (std::pmr::unordered_multiset): Likewise. * include/std/vector (std::pmr::vector): Likewise. * testsuite/21_strings/basic_string/types/pmr_typedefs.cc: New test. * testsuite/23_containers/deque/types/pmr_typedefs.cc: New test. * testsuite/23_containers/forward_list/pmr_typedefs.cc: New test. * testsuite/23_containers/list/pmr_typedefs.cc: New test. * testsuite/23_containers/map/pmr_typedefs.cc: New test. * testsuite/23_containers/multimap/pmr_typedefs.cc: New test. * testsuite/23_containers/multiset/pmr_typedefs.cc: New test. * testsuite/23_containers/set/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_map/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_multimap/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_multiset/pmr_typedefs.cc: New test. * testsuite/23_containers/unordered_set/pmr_typedefs.cc: New test. * testsuite/23_containers/vector/pmr_typedefs.cc: New test. * testsuite/28_regex/match_results/pmr_typedefs.cc: New test. diff --git a/libstdc++-v3/include/std/deque b/libstdc++-v3/include/std/deque index 896ec8aade0..d2c75f1f078 100644 --- a/libstdc++-v3/include/std/deque +++ b/libstdc++-v3/include/std/deque @@ -73,4 +73,18 @@ # include #endif +#if __cplusplus >= 201703L +namespace std _GLIBCXX_VISIBILITY(default) +{ +_GLIBCXX_BEGIN_NAMESPACE_VER
Re: [PATCH] i386: do not use SImode mul-highpart on 64-bit
On Thu, Aug 9, 2018 at 5:00 PM, Alexander Monakov wrote: > Hello, > > on x86-64, 32-bit division by constants uses mulsi3_highpart pattern that > turns into 'mull ' instruction with source implicitly in eax and > result in edx:eax. However, using 64-bit multiplication with zero-extended > source would be preferable, as the imulq instruction accepts the magic > multiplier as immediate if it fits into 31 bits, has fewer register allocation > constraints, and typically has better latency/throughput. > > Perhaps ideally we'd want expand_divmod to automatically choose this cheaper > variant on x86, but changing that appears to be too complicated. On the other > hand, we don't use mul_highpart patterns for anything else, so we can simply > expose only the DImode mul_highpart on x86-64. > > This patch does that by changing mode iterator so we offer only SImode > mul-highpart on 32-bit x86 and only DImode on 64-bit. > > Bootstrapped/regtested on x86-64, OK for trunk? > > Alexander > > PR target/82418 > * config/i386/i386.md (mul3_highpart): Use DWIH mode iterator > instead of SWI48. > > testsuite/ > * gcc.target/i386/pr82418.c: New test. OK. Thanks, Uros. > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index 73948c12618..10783d305d2 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -7792,16 +7792,16 @@ (define_insn "*mulqihi3_1" > (set_attr "mode" "QI")]) > > (define_expand "mul3_highpart" > - [(parallel [(set (match_operand:SWI48 0 "register_operand") > - (truncate:SWI48 > + [(parallel [(set (match_operand:DWIH 0 "register_operand") > + (truncate:DWIH > (lshiftrt: >(mult: > (any_extend: > - (match_operand:SWI48 1 "nonimmediate_operand")) > + (match_operand:DWIH 1 "nonimmediate_operand")) > (any_extend: > - (match_operand:SWI48 2 "register_operand"))) > + (match_operand:DWIH 2 "register_operand"))) >(match_dup 3 > - (clobber (match_scratch:SWI48 4)) > + (clobber (match_scratch:DWIH 4)) > (clobber (reg:CC FLAGS_REG))])] >"" >"operands[3] = GEN_INT (GET_MODE_BITSIZE (mode));") > diff --git a/gcc/testsuite/gcc.target/i386/pr82418.c > b/gcc/testsuite/gcc.target/i386/pr82418.c > index e69de29bb2d..95a506d5ccd 100644 > --- a/gcc/testsuite/gcc.target/i386/pr82418.c > +++ b/gcc/testsuite/gcc.target/i386/pr82418.c > @@ -0,0 +1,10 @@ > +/* PR target/82418 */ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-options "-O2" } */ > +/* { dg-final { scan-assembler "imul\[^\n\r]*1374389535" } } */ > + > +unsigned > +f1(unsigned x) > +{ > + return x / 100; > +}