Re: [RFC, PATCH 3/n] IPA C++ refactoring
On 08/01/2014 08:19 PM, Jeff Law wrote: > On 08/01/14 05:35, Martin Liška wrote: >> 2014-07-31 Martin Liska >> >> * cgraph.h (cgraph_node_set, varpool_node_set): Removed structs and all >> related functions that manipulated these data structures. >> (cgraph_new_nodes): vec replaces cgraph_node_set. >> * cgraphunit.c (cgraph_new_nodes): Likewise. >> (cgraph_process_new_functions): Likewise. >> (cgraph_node::add_new_function): Likewise. >> * ipa-inline-transform.c (can_remove_node_now_p_1): Likewise. >> * ipa-utils.c: Implementations of legacy data structures removed, >> data structure was replaced by a hash_map. >> * tree-emutls.c (tls_vars): hash_map replaces varpool_node_set. >> (emutls_index): New functions are used. >> (ipa_lower_emutls): Likewise. > Looks like a good cleanup. Just aone nit left with the most recent version > (the one where you fixed the indention issues pointed out by Jakub): > > >> >> >> symtab-node-removal.patch >> >> >> diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c >> index 89197c7..2c9cfe4 100644 >> --- a/gcc/tree-emutls.c >> +++ b/gcc/tree-emutls.c >> @@ -608,13 +581,20 @@ lower_emutls_phi_arg (gimple phi, unsigned int i, >> struct lower_emutls_data *d) >> } >> } >> >> +bool >> +reset_access (varpool_node * const &, tls_var_data *data, void *) >> +{ >> + data->access = NULL; >> + >> + return true; >> +} > Need a comment for this function. > > > Approved once you add that comment. Please post the final patch for archival > purposes. Hello, thank you for your feedback, there's fixed version of the patch that will be committed after Trevor's changes to hash-map. Martin > > Thanks, > Jeff > diff --git a/gcc/cgraph.h b/gcc/cgraph.h index d8651e2..edf3f7f 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1200,41 +1200,6 @@ public: unsigned calls_comdat_local : 1; }; -/* A cgraph node set is a collection of cgraph nodes. A cgraph node - can appear in multiple sets. */ -struct cgraph_node_set_def -{ - struct pointer_map_t *map; - vec nodes; -}; - -typedef cgraph_node_set_def *cgraph_node_set; -typedef struct varpool_node_set_def *varpool_node_set; - -class varpool_node; - -/* A varpool node set is a collection of varpool nodes. A varpool node - can appear in multiple sets. */ -struct varpool_node_set_def -{ - struct pointer_map_t * map; - vec nodes; -}; - -/* Iterator structure for cgraph node sets. */ -struct cgraph_node_set_iterator -{ - cgraph_node_set set; - unsigned index; -}; - -/* Iterator structure for varpool node sets. */ -struct varpool_node_set_iterator -{ - varpool_node_set set; - unsigned index; -}; - /* Structure containing additional information about an indirect call. */ struct GTY(()) cgraph_indirect_call_info @@ -1509,7 +1474,7 @@ enum cgraph_state }; extern enum cgraph_state cgraph_state; extern bool cgraph_function_flags_ready; -extern cgraph_node_set cgraph_new_nodes; +extern vec cgraph_new_nodes; extern GTY(()) struct asm_node *asm_nodes; extern GTY(()) int symtab_order; @@ -1615,24 +1580,7 @@ void record_references_in_initializer (tree, bool); /* In ipa.c */ bool symtab_remove_unreachable_nodes (bool, FILE *); -cgraph_node_set cgraph_node_set_new (void); -cgraph_node_set_iterator cgraph_node_set_find (cgraph_node_set, - cgraph_node *); -void cgraph_node_set_add (cgraph_node_set, cgraph_node *); -void cgraph_node_set_remove (cgraph_node_set, cgraph_node *); -void dump_cgraph_node_set (FILE *, cgraph_node_set); -void debug_cgraph_node_set (cgraph_node_set); -void free_cgraph_node_set (cgraph_node_set); void cgraph_build_static_cdtor (char which, tree body, int priority); - -varpool_node_set varpool_node_set_new (void); -varpool_node_set_iterator varpool_node_set_find (varpool_node_set, - varpool_node *); -void varpool_node_set_add (varpool_node_set, varpool_node *); -void varpool_node_set_remove (varpool_node_set, varpool_node *); -void dump_varpool_node_set (FILE *, varpool_node_set); -void debug_varpool_node_set (varpool_node_set); -void free_varpool_node_set (varpool_node_set); void ipa_discover_readonly_nonaddressable_vars (void); /* In predict.c */ @@ -1948,93 +1896,6 @@ cgraph_next_function_with_gimple_body (cgraph_node *node) /* Create a new static variable of type TYPE. */ tree add_new_static_var (tree type); -/* Return true if iterator CSI points to nothing. */ -static inline bool -csi_end_p (cgraph_node_set_iterator csi) -{ - return csi.index >= csi.set->nodes.length (); -} - -/* Advance iterator CSI. */ -static inline void -csi_next (cgraph_node_set_iterator *csi) -{ - csi->index++; -} - -/* Return the node pointed to by CSI. */ -static inline cgraph_node * -csi_node (cgraph_node_set_iterator csi) -{ - return csi.set->nodes[csi.index]; -} - -/* Return an iterator to the first node in SET. */ -static inline cgraph_node_set_iterator -csi_start (cgraph_node_set set) -{ - cgraph_node_set_iterator csi; - - csi
Re: [GSoC] checking for the loop parallelism
> Hi Roman, > > you can get this information from the isl_ast_build that was used when > generating a certain loop (you can access this isl_ast_build from the > callbacks isl_ast_build_set_before_each_for and > isl_ast_build_set_after_each_for). With isl_ast_build_get_schedule, you can > get an incomplete schedule (less dimensions then the schedule that you gave > to the isl ast generator). Specifically, it only contains the dimensions of > the current loop and all surrounding ones. Consequently the last dimension > in this incomplete schedule is the dimension you want to check for > parallelism. Hi Tobias, thank you! I've attached a patch, which contains the first draft of checking for the loop parallelism. If I'm not mistaken, the depth, which can be obtained from isl_ast_build, is only suitable for the incomplete schedule, which can be obtained using isl_ast_build_get_schedule. That's why the temporary implementation works with the incomplete schedule instead of the result from scop_get_transformed_schedule. I have a question about vect-pr43423.c. CLooG generates the following code from this example: vect-pr43423.c for (scat_1=0;scat_1<=min(mid_6-1,n_5-1);scat_1++) { (scat_1); (scat_1); } for (scat_1=max(0,mid_6);scat_1<=n_5-1;scat_1++) { (scat_1); (scat_1); } This loops can be parallelized, according to the description of pr43423: "... void foo(int n, int mid) { int i; for(i=0; i= c1 + 1) { S_6(c1); } else S_7(c1); S_8(c1); } I think it can't be parallelized. Maybe this example is no more suitable. What do you think about this? -- Cheers, Roman Gareev. Index: gcc/graphite-isl-ast-to-gimple.c === --- gcc/graphite-isl-ast-to-gimple.c(revision 213262) +++ gcc/graphite-isl-ast-to-gimple.c(working copy) @@ -435,7 +435,14 @@ redirect_edge_succ_nodup (next_e, after); set_immediate_dominator (CDI_DOMINATORS, next_e->dest, next_e->src); - /* TODO: Add checking for the loop parallelism. */ + if (flag_loop_parallelize_all) + { +isl_id *id = isl_ast_node_get_annotation (node_for); +gcc_assert (id); +if (isl_id_get_user (id) != NULL) + loop->can_be_parallel = true; +isl_id_free (id); + } return last_e; } @@ -834,6 +841,97 @@ return schedule_isl; } +/* Applies SCHEDULE to the in and out dimensions of the dependences + DEPS and return the resulting relation. */ + +static __isl_give isl_map * +apply_schedule_on_deps (__isl_keep isl_union_map *schedule, + __isl_keep isl_union_map *deps) +{ + isl_map *x; + isl_union_map *ux, *trans; + + trans = isl_union_map_copy (schedule); + trans = extend_schedule (trans); + ux = isl_union_map_copy (deps); + ux = isl_union_map_apply_domain (ux, isl_union_map_copy (trans)); + ux = isl_union_map_apply_range (ux, trans); + if (isl_union_map_is_empty (ux)) +{ + isl_union_map_free (ux); + return NULL; +} + x = isl_map_from_union_map (ux); + + return x; +} + +/* Return true when DEPS is non empty and the intersection of LEX with + the DEPS transformed by SCHEDULE is non empty. LEX is the relation + in which all the inputs before DEPTH occur at the same time as the + output, and the input at DEPTH occurs before output. */ + +static bool +carries_deps (__isl_keep isl_union_map *schedule, + __isl_keep isl_union_map *deps, + int depth) +{ + bool res; + int i; + isl_space *space; + isl_map *lex, *x; + isl_constraint *ineq; + + if (isl_union_map_is_empty (deps)) +return false; + + x = apply_schedule_on_deps (schedule, deps); + if (x == NULL) +return false; + space = isl_map_get_space (x); + space = isl_space_range (space); + lex = isl_map_lex_le (space); + space = isl_map_get_space (x); + ineq = isl_inequality_alloc (isl_local_space_from_space (space)); + + for (i = 0; i < depth - 1; i++) +lex = isl_map_equate (lex, isl_dim_in, i, isl_dim_out, i); + + /* in + 1 <= out */ + ineq = isl_constraint_set_coefficient_si (ineq, isl_dim_out, depth - 1, 1); + ineq = isl_constraint_set_coefficient_si (ineq, isl_dim_in, depth - 1, -1); + ineq = isl_constraint_set_constant_si (ineq, -1); + lex = isl_map_add_constraint (lex, ineq); + x = isl_map_intersect (x, lex); + res = !isl_map_is_empty (x); + + isl_map_free (x); + return res; +} + +/* This method is executed before the construction of a for node. */ +static __isl_give isl_id * +ast_build_before_for (__isl_keep isl_ast_build *build, void *user) +{ + scop_p scop = (scop_p) user; + isl_ast_build *pointer = NULL; + isl_union_map *schedule = isl_ast_build_get_schedule (build); + isl_space *schedule_space = isl_ast_build_get_schedule_space (build); + int dimension = isl_space_dim (schedule_space, isl_dim_out) - 1; + int res = (carries_deps (schedule, scop->must_raw, dimension) +|| carries_deps (schedule, scop->may_raw, dimension) +
Add -Wsuggest-final-warnings and -Wsuggest-final-methods warnings
Hi, this patch adds warnings -Wsuggest-final-types and -Wsuggest-final-methods that output warnings when adding C++ final keyword would enable devirtualizations. This warning is expected to be taken as a hint by the developers who can annotate sources where it makes sense from design standpoint. The warning works both with and without LTO, but it is a lot more accurate with LTO. About 8000 warings are output for libxul build, so I added code sorting them by importance. Type final specifiers are suggested first, method later (because making type final makes method annotations unnecesary). Warnings output for firefox are placed here http://kam.mff.cuni.cz/~hubicka/final-warnings.txt I can also add warnings for all types with no derived types and all virtual methods with no overriders, but I am not sure if that would be of any use. The patch also speeds up the type inheritance analysis that got quite slow on Firefox after introducing speculative contextes (we now derrive a lot of speculative info we didn't before). I think i will need to reorganize how speculation is handled andseparate fll and speculative lists as one tends to be long and others short but many. Bootstrapped/regtested x86_64-linux, I am retesting after a last minute change and intend to commit soon. Honza * doc/invoke.texi (Wsuggest-final-types, Wsuggest-final-methods): Document. * ipa-devirt.c: Include hash-map.h (struct polymorphic_call_target_d): Add type_warning and decl_warning. (clear_speculation): Break out of ... (get_class_context): ... here; speed up handling obviously useless speculations. (odr_type_warn_count, decl_warn_count): New structures. (final_warning_record): New structure. (final_warning_records): New static variable. (possible_polymorphic_call_targets): Cleanup handling of speculative info; do not build speculation when user do not care; record info about warnings when asked for. (add_decl_warning): New function. (type_warning_cmp): New function. (decl_warning_cmp): New function. (ipa_devirt): Handle -Wsuggest-final-methods and -Wsuggest-final-types. (gate): Enable pass when warnings are requested. * common.opt (Wsuggest-final-types, Wsuggest-final-methods): New options. * g++.dg/warn/Wsuggest-final.C: New testcase. * g++.dg/ipa/devirt-34.C: Fix. Index: doc/invoke.texi === --- doc/invoke.texi (revision 213406) +++ doc/invoke.texi (working copy) @@ -271,6 +271,7 @@ Objective-C and Objective-C++ Dialects}. -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol +-Wsuggest-final-types @gol -Wsuggest-final-methods @gol -Wmissing-format-attribute @gol -Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol -Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol @@ -4193,6 +4194,25 @@ case, and some functions for which @code appropriate may not be detected. @end table +@item -Wsuggest-final-types +@opindex Wno-suggest-final-types +@opindex Wsuggest-final-types +Warn about types with virtual methods where code quality would be improved +if the type was declared with C++11 final specifier, or, if possible, +declared in anonymous namespace. This allows GCC to devritualize more aggressively +the polymorphic calls. This warning is more effective with link time optimization, +where the information about the class hiearchy is more complete. + +@item -Wsuggest-final-methods +@opindex Wno-suggest-final-methods +@opindex Wsuggest-final-methods +Warn about virtual methods where code quality would be improved if the method +was declared with C++11 final specifier, or, if possible, its type was declared +in the anonymous namespace or with final specifier. This warning is more +effective with link time optimization, where the information about the class +hiearchy graph is more complete. It is recommended to first consider suggestions +of @option{-Wsuggest-final-types} and then rebuild with new annotations. + @item -Warray-bounds @opindex Wno-array-bounds @opindex Warray-bounds @@ -9601,7 +9621,7 @@ before applying @option{--param inline-u @item inline-unit-growth Specifies maximal overall growth of the compilation unit caused by inlining. The default value is 30 which limits unit growth to 1.3 times the original -size. Cold functions (either marked cold via an attribibute or by profile +size. Cold functions (either marked cold via an attribute or by profile feedback) are not accounted into the unit size. @item ipcp-unit-growth Index: testsuite/g++.dg/warn/Wsuggest-final.C === --- testsuite/g++.dg/warn/Wsuggest-final.C
Re: [GSoC] checking for the loop parallelism
On 02/08/2014 11:49, Roman Gareev wrote: Hi Roman, > >you can get this information from the isl_ast_build that was used when >generating a certain loop (you can access this isl_ast_build from the >callbacks isl_ast_build_set_before_each_for and >isl_ast_build_set_after_each_for). With isl_ast_build_get_schedule, you can >get an incomplete schedule (less dimensions then the schedule that you gave >to the isl ast generator). Specifically, it only contains the dimensions of >the current loop and all surrounding ones. Consequently the last dimension >in this incomplete schedule is the dimension you want to check for >parallelism. Hi Tobias, thank you! I've attached a patch, which contains the first draft of checking for the loop parallelism. If I'm not mistaken, the depth, which can be obtained from isl_ast_build, is only suitable for the incomplete schedule, which can be obtained using isl_ast_build_get_schedule. That's why the temporary implementation works with the incomplete schedule instead of the result from scop_get_transformed_schedule. Yes. Using the isl_ast_build_get_schedule is the right approach. I have a question about vect-pr43423.c. CLooG generates the following code from this example: vect-pr43423.c for (scat_1=0;scat_1<=min(mid_6-1,n_5-1);scat_1++) { (scat_1); (scat_1); } for (scat_1=max(0,mid_6);scat_1<=n_5-1;scat_1++) { (scat_1); (scat_1); } This loops can be parallelized, according to the description of pr43423: "... void foo(int n, int mid) { int i; for(i=0; i= c1 + 1) { S_6(c1); } else S_7(c1); S_8(c1); } I think it can't be parallelized. This looks very similar to what we reported to the isl mailing list. It is definitely not the best test case for the parallelism patch. In fact, I doubt this requires the parallelism test at all. You should have a look at the following test case for openmp tests: libgomp/testsuite/libgomp.graphite I reply to the isl mailing list regarding the problem reported above. Index: gcc/graphite-isl-ast-to-gimple.c === --- gcc/graphite-isl-ast-to-gimple.c(revision 213262) +++ gcc/graphite-isl-ast-to-gimple.c(working copy) @@ -435,7 +435,14 @@ redirect_edge_succ_nodup (next_e, after); set_immediate_dominator (CDI_DOMINATORS, next_e->dest, next_e->src); - /* TODO: Add checking for the loop parallelism. */ + if (flag_loop_parallelize_all) + { +isl_id *id = isl_ast_node_get_annotation (node_for); +gcc_assert (id); +if (isl_id_get_user (id) != NULL) + loop->can_be_parallel = true; I would prefer if we actually use a user structure which contains an isParallel flag. The use of the presence of isl_id to show parallelism is a little indirect. +static __isl_give isl_map * +apply_schedule_on_deps (__isl_keep isl_union_map *schedule, + __isl_keep isl_union_map *deps) +{ + isl_map *x; + isl_union_map *ux, *trans; + + trans = isl_union_map_copy (schedule); + trans = extend_schedule (trans); + ux = isl_union_map_copy (deps); + ux = isl_union_map_apply_domain (ux, isl_union_map_copy (trans)); + ux = isl_union_map_apply_range (ux, trans); + if (isl_union_map_is_empty (ux)) +{ + isl_union_map_free (ux); + return NULL; +} + x = isl_map_from_union_map (ux); + + return x; +} + +/* Return true when DEPS is non empty and the intersection of LEX with + the DEPS transformed by SCHEDULE is non empty. LEX is the relation + in which all the inputs before DEPTH occur at the same time as the + output, and the input at DEPTH occurs before output. */ + +static bool +carries_deps (__isl_keep isl_union_map *schedule, + __isl_keep isl_union_map *deps, + int depth) +{ + bool res; + int i; + isl_space *space; + isl_map *lex, *x; + isl_constraint *ineq; + + if (isl_union_map_is_empty (deps)) +return false; + + x = apply_schedule_on_deps (schedule, deps); + if (x == NULL) +return false; + space = isl_map_get_space (x); + space = isl_space_range (space); + lex = isl_map_lex_le (space); + space = isl_map_get_space (x); + ineq = isl_inequality_alloc (isl_local_space_from_space (space)); + + for (i = 0; i < depth - 1; i++) +lex = isl_map_equate (lex, isl_dim_in, i, isl_dim_out, i); + + /* in + 1 <= out */ + ineq = isl_constraint_set_coefficient_si (ineq, isl_dim_out, depth - 1, 1); + ineq = isl_constraint_set_coefficient_si (ineq, isl_dim_in, depth - 1, -1); + ineq = isl_constraint_set_constant_si (ineq, -1); + lex = isl_map_add_constraint (lex, ineq); + x = isl_map_intersect (x, lex); + res = !isl_map_is_empty (x); + + isl_map_free (x); + return res; +} Why did you copy this code, instead of exposing the carries_deps functions from graphite-dependences? + +/* This method is executed before the construction of a for node. */ +static __isl_give isl_id * +ast_build_before_for (__isl_keep isl_ast_build *build, void *user)
Re: [PATCH] convert many pointer_map to hash_map
On Fri, Aug 01, 2014 at 12:52:08PM +0200, Richard Biener wrote: > On Fri, Aug 1, 2014 at 12:34 PM, wrote: > > From: Trevor Saunders > > > > Hi, > > > > This patch replaces a bunch of usage of pointer_map with hash_map. It also > > adds an overload to hash_map::traverse that allows modifying the value, and > > a > > remove method. > > > > bootstrapped + regtested on x86_64-unknown-linux-gnu, ok? > > Ok. committed as r213517 thanks for the review. Trev > > Thanks, > Richard. > > > Trev > > > > c-family/ > > > > * cilk.c: Use hash_map instead of pointer_map. > > > > c/ > > > > * c-typeck.c: Use hash_map instead of pointer_map. > > > > cp/ > > > > * optimize.c, semantics.c: Use hash_map instead of pointer_map. > > > > gcc/ > > > > * hash-map.h (default_hashmap_traits::mark_key_deleted): > > Fix cast. > > (hash_map::remove): New method. > > (hash_map::traverse): New method. > > * cgraph.h, except.c, except.h, gimple-ssa-strength-reduction.c, > > ipa-utils.c, lto-cgraph.c, lto-streamer.h, omp-low.c, predict.c, > > tree-cfg.c, tree-cfgcleanup.c, tree-eh.c, tree-eh.h, tree-inline.c, > > tree-inline.h, tree-nested.c, tree-sra.c, tree-ssa-loop-im.c, > > tree-ssa-loop-ivopts.c, tree-ssa-reassoc.c, tree-ssa-structalias.c, > > tree-ssa.c, tree-ssa.h, var-tracking.c: Use hash_map instead of > > pointer_map. > > diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c > > index b864bb1..e0d1141 100644 > > --- a/gcc/c-family/cilk.c > > +++ b/gcc/c-family/cilk.c > > @@ -64,7 +64,7 @@ struct wrapper_data > >/* Containing function. */ > >tree context; > >/* Disposition of all variables in the inner statement. */ > > - struct pointer_map_t *decl_map; > > + hash_map *decl_map; > >/* True if this function needs a static chain. */ > >bool nested; > >/* Arguments to be passed to wrapper function, currently a list. */ > > @@ -335,12 +335,11 @@ create_cilk_helper_decl (struct wrapper_data *wd) > > > > /* A function used by walk tree to find wrapper parms. */ > > > > -static bool > > -wrapper_parm_cb (const void *key0, void **val0, void *data) > > +bool > > +wrapper_parm_cb (tree const &key0, tree *val0, wrapper_data *wd) > > { > > - struct wrapper_data *wd = (struct wrapper_data *) data; > > - tree arg = * (tree *)&key0; > > - tree val = (tree)*val0; > > + tree arg = key0; > > + tree val = *val0; > >tree parm; > > > >if (val == error_mark_node || val == arg) > > @@ -387,7 +386,7 @@ build_wrapper_type (struct wrapper_data *wd) > >wd->parms = NULL_TREE; > >wd->argtypes = void_list_node; > > > > - pointer_map_traverse (wd->decl_map, wrapper_parm_cb, wd); > > + wd->decl_map->traverse (wd); > >gcc_assert (wd->type != CILK_BLOCK_FOR); > > > >/* Now build a function. > > @@ -452,25 +451,22 @@ copy_decl_for_cilk (tree decl, copy_body_data *id) > > > > /* Copy all local variables. */ > > > > -static bool > > -for_local_cb (const void *k_v, void **vp, void *p) > > +bool > > +for_local_cb (tree const &k, tree *vp, copy_body_data *id) > > { > > - tree k = *(tree *) &k_v; > > - tree v = (tree) *vp; > > + tree v = *vp; > > > >if (v == error_mark_node) > > -*vp = copy_decl_no_change (k, (copy_body_data *) p); > > +*vp = copy_decl_no_change (k, id); > >return true; > > } > > > > /* Copy all local declarations from a _Cilk_spawned function's body. */ > > > > -static bool > > -wrapper_local_cb (const void *k_v, void **vp, void *data) > > +bool > > +wrapper_local_cb (tree const &key, tree *vp, copy_body_data *id) > > { > > - copy_body_data *id = (copy_body_data *) data; > > - tree key = *(tree *) &k_v; > > - tree val = (tree) *vp; > > + tree val = *vp; > > > >if (val == error_mark_node) > > *vp = copy_decl_for_cilk (key, id); > > @@ -514,8 +510,11 @@ cilk_outline (tree inner_fn, tree *stmt_p, void *w) > >insert_decl_map (&id, wd->block, DECL_INITIAL (inner_fn)); > > > >/* We don't want the private variables any more. */ > > - pointer_map_traverse (wd->decl_map, nested ? for_local_cb : > > wrapper_local_cb, > > - &id); > > + if (nested) > > +wd->decl_map->traverse (&id); > > + else > > +wd->decl_map->traverse (&id); > > + > >walk_tree (stmt_p, copy_tree_body_r, (void *) &id, NULL); > > > >/* See if this function can throw or calls something that should > > @@ -576,7 +575,7 @@ init_wd (struct wrapper_data *wd, enum cilk_block_type > > type) > >wd->type = type; > >wd->fntype = NULL_TREE; > >wd->context = current_function_decl; > > - wd->decl_map = pointer_map_create (); > > + wd->decl_map = new hash_map; > >/* _Cilk_for bodies are always nested. Others start off as > > normal functions. */ > >wd->nested = (type == CILK_BLOCK_FOR); > > @@ -590,7 +589,7 @@ init_wd (struct wrapper_data *wd, enum cilk_block_type > > type) > > static void > > free_wd (struct wrapper_dat
Re: [C++ Patch] PR 15339
OK. Jason
Re: [PATCH] convert many pointer_map to hash_map
On Sat, 2014-08-02 at 07:34 -0400, Trevor Saunders wrote: > On Fri, Aug 01, 2014 at 12:52:08PM +0200, Richard Biener wrote: > > On Fri, Aug 1, 2014 at 12:34 PM, wrote: > > > From: Trevor Saunders > > > > > > Hi, > > > > > > This patch replaces a bunch of usage of pointer_map with hash_map. It > > > also > > > adds an overload to hash_map::traverse that allows modifying the value, > > > and a > > > remove method. > > > > > > bootstrapped + regtested on x86_64-unknown-linux-gnu, ok? > > > > Ok. > > committed as r213517 thanks for the review. > > Trev > Either r213517 or r213516 is causing the following on my sh-elf setup (i686 host) when doing 'make all': libtool: compile: /home/user/code/gcc/gcc-trunk-build-sh-elf/./gcc/xgcc -shared-libgcc -B/home/user/code/gcc/gcc-trunk-build-sh-elf/./gcc -nostdinc++ -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc ++-v3/src -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc ++-v3/src/.libs -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/libsupc ++/.libs -B/usr/local/sh-elf/bin/ -B/usr/local/sh-elf/lib/ -isystem /usr/local/sh-elf/include -isystem /usr/local/sh-elf/sys-include -I/home/user/code/gcc/gcc-trunk/libstdc++-v3/../libgcc -I/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc ++-v3/include/sh-elf -I/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/include -I/home/user/code/gcc/gcc-trunk/libstdc++-v3/libsupc++ -std=gnu++11 -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=fstream-inst.lo -g -O2 -c ../../../../../gcc-trunk/libstdc ++-v3/src/c++11/fstream-inst.cc -o fstream-inst.o cc1plus: out of memory allocating 2129023400 bytes after a total of 1589248 bytes make[5]: *** [fstream-inst.lo] Error 1 make[5]: Leaving directory `/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/src/c ++11' Cheers, Oleg
Re: [PATCH] convert many pointer_map to hash_map
On Sat, Aug 02, 2014 at 05:38:42PM +0200, Oleg Endo wrote: > On Sat, 2014-08-02 at 07:34 -0400, Trevor Saunders wrote: > > On Fri, Aug 01, 2014 at 12:52:08PM +0200, Richard Biener wrote: > > > On Fri, Aug 1, 2014 at 12:34 PM, wrote: > > > > From: Trevor Saunders > > > > > > > > Hi, > > > > > > > > This patch replaces a bunch of usage of pointer_map with hash_map. It > > > > also > > > > adds an overload to hash_map::traverse that allows modifying the value, > > > > and a > > > > remove method. > > > > > > > > bootstrapped + regtested on x86_64-unknown-linux-gnu, ok? > > > > > > Ok. > > > > committed as r213517 thanks for the review. > > > > Trev > > > > Either r213517 or r213516 is causing the following on my sh-elf setup > (i686 host) when doing 'make all': I'd guess that its r213517, and for at least one of the hash maps we're leaking what the entries point at. Can you try compiling that file under valgrind? if not I can try in a couple hours. Trev > > libtool: compile: /home/user/code/gcc/gcc-trunk-build-sh-elf/./gcc/xgcc > -shared-libgcc -B/home/user/code/gcc/gcc-trunk-build-sh-elf/./gcc > -nostdinc++ -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc > ++-v3/src -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc > ++-v3/src/.libs > -L/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/libsupc > ++/.libs -B/usr/local/sh-elf/bin/ -B/usr/local/sh-elf/lib/ > -isystem /usr/local/sh-elf/include > -isystem /usr/local/sh-elf/sys-include > -I/home/user/code/gcc/gcc-trunk/libstdc++-v3/../libgcc > -I/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc > ++-v3/include/sh-elf > -I/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/include > -I/home/user/code/gcc/gcc-trunk/libstdc++-v3/libsupc++ -std=gnu++11 > -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi > -fdiagnostics-show-location=once -ffunction-sections -fdata-sections > -frandom-seed=fstream-inst.lo -g -O2 -c ../../../../../gcc-trunk/libstdc > ++-v3/src/c++11/fstream-inst.cc -o fstream-inst.o > > cc1plus: out of memory allocating 2129023400 bytes after a total of > 1589248 bytes > make[5]: *** [fstream-inst.lo] Error 1 > make[5]: Leaving directory > `/home/user/code/gcc/gcc-trunk-build-sh-elf/sh-elf/libstdc++-v3/src/c > ++11' > > Cheers, > Oleg >
Re: [PATCH][testsuite] Don't run cproj-fails-with-broken-glibc.c for broken glibc
On 01-08-14 12:35, Rainer Orth wrote: Hi Tom, The test-case cproj-fails-with-broken-glibc.c does not work with broken glibcs, as the header comment mentions: ... Check the runtime behavior of the C library's cproj() function and whether it follows the standard. Versions of GLIBC through 2.11.1 had an incorrect implementation which will conflict with GCC's builtin cproj(). GLIBC 2.12+ should be okay. ... This patch skips the test for known broken glibcs. OK for trunk? I'm not at all happy with this patch: unless absolutely necessary, we shouldn't check for version numbers when there's any way to check for working/broken features instead. I'm all for keeping testsuite results clean, but this seems like a total corner case to me. Which distributions do still use affected glibc versions? Ubuntu 10.04 LTS That test, even if we go the glibc version route, needs to be XFAILed instead of requiring the working version. Apart from that, new effective-target keywords need documenting in doc/sourcebuild.texi. I've made it an xfail, and added documentation in attached follow-up patch. OK? Or do we go with the removal suggestion of Mike? Thanks, - Tom 2014-08-01 Tom de Vries * gcc.dg/cproj-fails-with-broken-glibc.c: Use xfail for broken glibc version instead of required-target. * lib/target-supports.exp (check_effective_target_not_glibc_2_11_or_earlier): Replace by ... (check_effective_target_glibc_2_11_or_earlier): ... this. * doc/sourcebuild.texi (glibc, glibc_2_12_or_later) (glibc_2_11_or_earlier): Document effective-target keywords. diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index 39152df..0793f80 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1804,6 +1804,15 @@ Target is a VxWorks RTP. @item wchar Target supports wide characters. + +@item glibc +Target supports glibc + +@item glibc_2_12_or_later +Target supports glibc 2.12 or later + +@item glibc_2_11_or_earlier +Target supports glibc 2.11 or earlier @end table @subsubsection Other attributes diff --git a/gcc/testsuite/gcc.dg/cproj-fails-with-broken-glibc.c b/gcc/testsuite/gcc.dg/cproj-fails-with-broken-glibc.c index 1df29f9..fc37fac 100644 --- a/gcc/testsuite/gcc.dg/cproj-fails-with-broken-glibc.c +++ b/gcc/testsuite/gcc.dg/cproj-fails-with-broken-glibc.c @@ -7,11 +7,10 @@ Origin: Kaveh R. Ghazi, April 20, 2010. */ -/* { dg-do run } */ +/* { dg-do run { xfail glibc_2_11_or_earlier } } */ /* { dg-options "-fno-builtin-cproj" } */ /* { dg-add-options c99_runtime } */ /* { dg-require-effective-target c99_runtime } */ -/* { dg-require-effective-target not_glibc_2_11_or_earlier } */ extern void abort(void); extern void exit(int); diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index cbe2930..7157d2a 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -5061,19 +5061,19 @@ proc check_effective_target_glibc_2_12_or_later {} { }] } -# Return true if this is a not a glibc 2.11 or earlier target. +# Return true if this is a glibc 2.11 or earlier target. -proc check_effective_target_not_glibc_2_11_or_earlier {} { +proc check_effective_target_glibc_2_11_or_earlier {} { if { ![check_effective_target_glibc] } { return 1 } if { [check_effective_target_glibc_2_12_or_later] } { - return 1 + return 0 } -return 0 +return 1 } # Return true if this is NOT a Bionic target. -- 1.9.1
Re: [PATCH] convert many pointer_map to hash_map
On 02 Aug 2014, at 17:58, Trevor Saunders wrote: > On Sat, Aug 02, 2014 at 05:38:42PM +0200, Oleg Endo wrote: >> On Sat, 2014-08-02 at 07:34 -0400, Trevor Saunders wrote: >>> On Fri, Aug 01, 2014 at 12:52:08PM +0200, Richard Biener wrote: On Fri, Aug 1, 2014 at 12:34 PM, wrote: > From: Trevor Saunders > > Hi, > > This patch replaces a bunch of usage of pointer_map with hash_map. It > also > adds an overload to hash_map::traverse that allows modifying the value, > and a > remove method. > > bootstrapped + regtested on x86_64-unknown-linux-gnu, ok? Ok. >>> >>> committed as r213517 thanks for the review. >>> >>> Trev >>> >> >> Either r213517 or r213516 is causing the following on my sh-elf setup >> (i686 host) when doing 'make all': > > I'd guess that its r213517, and for at least one of the hash maps we're > leaking what the entries point at. Can you try compiling that file > under valgrind? if not I can try in a couple hours. I think you'll be faster than me. Cheers, Oleg
[GSoC][match-and-simplify] use dt_simplify::capture_max
* genmatch.c (dt_simplify::gen_gimple): Use dt_simplify::capture_max. (dt_simplify::gen_generic): Likewise. Thanks, Prathamesh Index: genmatch.c === --- genmatch.c (revision 213343) +++ genmatch.c (working copy) @@ -1473,7 +1473,7 @@ dt_simplify::gen_gimple (FILE *f) fprintf (f, "/* simplify %u */\n", pattern_no); fprintf (f, "{\n"); - fprintf (f, "tree captures[4] ATTRIBUTE_UNUSED = {};\n"); + fprintf (f, "tree captures[%u] ATTRIBUTE_UNUSED = {};\n", dt_simplify::capture_max); for (unsigned i = 0; i < dt_simplify::capture_max; ++i) if (indexes[i]) @@ -1538,7 +1538,7 @@ dt_simplify::gen_generic (FILE *f) fprintf (f, "/* simplify %u */\n", pattern_no); fprintf (f, "{\n"); - fprintf (f, "tree captures[4] ATTRIBUTE_UNUSED = {};\n"); + fprintf (f, "tree captures[%u] ATTRIBUTE_UNUSED = {};\n", dt_simplify::capture_max); for (unsigned i = 0; i < dt_simplify::capture_max; ++i) if (indexes[i])
Re: [PATCH] Keep patch file permissions in mklog
On 01-08-14 09:18, Yury Gribov wrote: On 08/01/2014 10:52 AM, Tom de Vries wrote: This patch adds a script contrib/mklog-in-patch, which uses mklog to generate the skeleton log, but generates the log at the start of the patch as mklog did before (which is how I like to use it). Yeah, we had some argument about this but kind of agreed that separate log is preferred. I can also try to add an --inline option to mklog instead. I'd prefer this. This patch implements an --inline option to mklog. I've used the --inline option to prepend the skeleton log to this patch. OK for trunk? Thanks, - Tom 2014-08-02 Tom de Vries * mklog: Add --inline option. diff --git a/contrib/mklog b/contrib/mklog index 3d17dc5..ba075cf 100755 --- a/contrib/mklog +++ b/contrib/mklog @@ -56,19 +56,27 @@ if (-d "$gcc_root/.git") { # Program starts here. You should not need to edit anything below this # line. #- -if ($#ARGV != 0) { +$inline = 0; +if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq "--inline")) { + $diff = $ARGV[1]; + $inline = 1; + if ($diff eq "-") { + die "Reading from - and using -i are not compatible"; + } +} elsif ($#ARGV != 0) { $prog = `basename $0`; chop ($prog); print <', $tmp) or die "Could not open temp file"; +} else { +*FILE1 = STDOUT; +} + +# Print the log foreach my $clname (keys %cl_entries) { - print "$clname:\n\n$hdrline\n\n$cl_entries{$clname}\n"; + print FILE1 "$clname:\n\n$hdrline\n\n$cl_entries{$clname}\n"; +} + + +# Prepend the log to the patch +if ($inline) { + close (FILE1); + + system ("cat $diff >>$tmp") == 0 + or die "Could not append patch to temp file"; + + # We're using cat rather than move, to keep permissions on $diff the same. + system ("cat $tmp >$diff") == 0 + or die "Could not move temp file to patch file"; + + unlink ($tmp) == 1 or die "Could not remove temp file"; } exit 0; -- 1.9.1
Re: [PATCH] add hash_set
On Tue, Jul 29, 2014 at 5:50 AM, wrote: > From: Trevor Saunders > > Hi, > > this adds a hash_set wrapper around hash_table, and then replaces usage of > pointer_set with it. > > bootstrapped +regtested on x86_64-unknown-linux-gnu, ok? > > Trev > > ada/ > > * gcc-interface/trans.c: Use hash_set instead of pointer_set. > > c-family/ > > * c-gimplify.c: Use hash_set instead of pointer_set. > > c/ > > * c-decl.c: Use hash_set instead of pointer_set. > > cp/ > > * class.c, cp-gimplify.c, cp-tree.h, decl.c, decl2.c, error.c, > method.c, name-lookup.c, pt.c, semantics.c, tree.c: Use hash_set > instead of pointer_set. > > fortran/ > > * openmp.c, trans-decl.c: Use hash_set instead of pointer_set. > > gcc/ > > * hash-set.h: new File. > * cfgexpand.c, cfgloop.c, cgraph.c, cgraphbuild.c, cgraphunit.c, > cprop.c, cse.c, gimple-walk.c, gimple-walk.h, gimplify.c, godump.c, > ipa-devirt.c, ipa-pure-const.c, ipa-visibility.c, ipa.c, lto-cgraph.c, > lto-streamer-out.c, stmt.c, tree-cfg.c, tree-core.h, tree-eh.c, > tree-inline.c, tree-inline.h, tree-nested.c, tree-pretty-print.c, > tree-ssa-loop-niter.c, tree-ssa-phiopt.c, tree-ssa-threadedge.c, > tree-ssa-uninit.c, tree.c, tree.h, value-prof.c, varasm.c, > varpool.c: Use hash_set instead of pointer_set. This changelog is less than useful and really does not follow the GCC/GNU coding style. Please expand and place all functions which are changed in it. I use ChangeLog entries when merging in code to see which of the merged code I need to pick up. Thanks, Andrew Pinski > > lto/ > > * lto-partition.c, lto-partition.h: Use hash_set instead of > pointer_set. > diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c > index f42ac7f..6961838 100644 > --- a/gcc/ada/gcc-interface/trans.c > +++ b/gcc/ada/gcc-interface/trans.c > @@ -36,7 +36,7 @@ > #include "output.h" > #include "libfuncs.h" /* For set_stack_check_libfunc. */ > #include "tree-iterator.h" > -#include "pointer-set.h" > +#include "hash-set.h" > #include "gimple-expr.h" > #include "gimplify.h" > #include "bitmap.h" > @@ -3054,7 +3054,7 @@ struct nrv_data >bitmap nrv; >tree result; >Node_Id gnat_ret; > - struct pointer_set_t *visited; > + hash_set *visited; > }; > > /* Return true if T is a Named Return Value. */ > @@ -3188,7 +3188,7 @@ finalize_nrv_r (tree *tp, int *walk_subtrees, void > *data) >/* Avoid walking into the same tree more than once. Unfortunately, we > can't just use walk_tree_without_duplicates because it would only > call us for the first occurrence of NRVs in the function body. */ > - if (pointer_set_insert (dp->visited, *tp)) > + if (dp->visited->add (*tp)) > *walk_subtrees = 0; > >return NULL_TREE; > @@ -3328,7 +3328,7 @@ finalize_nrv_unc_r (tree *tp, int *walk_subtrees, void > *data) >/* Avoid walking into the same tree more than once. Unfortunately, we > can't just use walk_tree_without_duplicates because it would only > call us for the first occurrence of NRVs in the function body. */ > - if (pointer_set_insert (dp->visited, *tp)) > + if (dp->visited->add (*tp)) > *walk_subtrees = 0; > >return NULL_TREE; > @@ -3376,13 +3376,13 @@ finalize_nrv (tree fndecl, bitmap nrv, vec va_gc> *other, Node_Id gnat_ret >data.nrv = nrv; >data.result = DECL_RESULT (fndecl); >data.gnat_ret = gnat_ret; > - data.visited = pointer_set_create (); > + data.visited = new hash_set; >if (TYPE_RETURN_UNCONSTRAINED_P (TREE_TYPE (fndecl))) > func = finalize_nrv_unc_r; >else > func = finalize_nrv_r; >walk_tree (&DECL_SAVED_TREE (fndecl), func, &data, NULL); > - pointer_set_destroy (data.visited); > + delete data.visited; > } > > /* Return true if RET_VAL can be used as a Named Return Value for the > diff --git a/gcc/c-family/c-gimplify.c b/gcc/c-family/c-gimplify.c > index 2b5ce5b..4898217 100644 > --- a/gcc/c-family/c-gimplify.c > +++ b/gcc/c-family/c-gimplify.c > @@ -74,7 +74,7 @@ along with GCC; see the file COPYING3. If not see > static tree > ubsan_walk_array_refs_r (tree *tp, int *walk_subtrees, void *data) > { > - struct pointer_set_t *pset = (struct pointer_set_t *) data; > + hash_set *pset = (hash_set *) data; > >/* Since walk_tree doesn't call the callback function on the decls > in BIND_EXPR_VARS, we have to walk them manually. */ > @@ -116,10 +116,9 @@ c_genericize (tree fndecl) > >if (flag_sanitize & SANITIZE_BOUNDS) > { > - struct pointer_set_t *pset = pointer_set_create (); > - walk_tree (&DECL_SAVED_TREE (fndecl), ubsan_walk_array_refs_r, pset, > -pset); > - pointer_set_destroy (pset); > + hash_set pset; > + walk_tree (&DECL_SAVED_TREE (fndecl), ubsan_walk_array_refs_r, &pset, > +&pset); > } > >/* Dump the C-specific tree IR. */
Re: [PATCH] libjava/classpath/native/jni/java-lang/java_lang_VMProcess.c: Be sure 'errbuf' always be zero terminated.
Excuse me, after tried, I still did not know hot to build the source code for "x86_64-unknown-linux-gnu/32/libjava/classpath/native/jni". What I have done is: - ../gcc/configure --enable-core-jni --enable-languages=c,c++,java make all-target-libjava - also try "../gcc/configure && make", but get same result. - I also enable JNIDIRS in "x86_64-unknown-linux-gnu/libjava/classpath /native/jni/Makefile" manually, but still no effect. Welcome any ideas, suggestions or completions for it, thank. Also sorry, I did not finish sending patch v2 for it within 2014-08-03, one excuse is: for each complete building, it needs 12-15 hours under my laptop. So next, I shall buy a PC for it (also for linux kernel). Thanks. On 08/01/2014 01:19 PM, Chen Gang wrote: > > Sorry for no testsuite, I shall send patch v2 for it after finish > related testsuite within this week end (2014-08-03). > > And the patch v2 also need cc to java-patc...@gcc.gnu.org > > Thanks. > > On 07/31/2014 12:59 PM, Chen Gang wrote: >> >> >> On 07/31/2014 12:10 PM, Jeff Law wrote: >>> On 07/30/14 09:01, Chen Gang wrote: Hello All: I shall stop making this kind of patch, next. The reason is that I worry about what I have done have negative effect to others. And next, I shall try to send another kinds of patches for gcc when I have time. Many persons or companies use open source who never give thanks or contribution back to open source. But open source (especially, fundamental software) still provide common contributions to outside. What I have done is only for contribution back to open source, so I can understand none-reply from open source (at least, it is not the excuse to let myself stop). But what I worry about is whether bother others. >>> I don't think you've have any kind of negative impact. GCC developers >>> tend to be a bit more conservative and try not to change code just for >>> the sake of changing code. Thus we tend to want to have a clearer >>> understanding of why a particular change needs to be made. >>> >>> It's also the case that we tend to look more closely at patches from >>> relatively new developers simply because we don't have a long history of >>> interactions that have built trust over time. >>> >>> So, just to be clear, I don't think you're bothering anyone and I would >>> recommend you continue to analyze code and send patches. >>> >> >> OK, thank you for your encouragement. And I shall continue to send this >> kind of patches. >> >>> And sorry for telling you everything goes to gcc-patches. I often >>> forget there's a separate java patch list -- and more generally for the >>> runtime libraries, we're often a downstream code consumer. Thus a >>> proposed change in some of the runtime libraries may need to be sent to >>> other projects (Classpath is a good example). >>> >> >> OK, and I shall notice about it next time (send related patches to >> correct mailing list). >> >> For me, it will be good idea to have a related document for these >> mailing list intruduction (maybe it has, and I shall try to find it). >> >> >> Thanks. >> > -- Chen Gang Open, share, and attitude like air, water, and life which God blessed