Re: [cxx-conversion] various hash tables, part 1/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote:
On Sat, Dec 1, 2012 at 8:47 PM, Lawrence Crowl cr...@googlers.com wrote:
+inline bool
+attribute_hasher::equal (const value_type *spec, const compare_type
*str)
+{
+ return (!strncmp (spec-name, str-str, str-length)
I have a slight preference for strncmp() == 0. It's easier to read (I
realize that you are just cut-n-pasting old code here :)
Done.
@@ -76,13 +82,11 @@ bitmap_descriptor (const char *file, int
loc.function = function;
loc.line = line;
- if (!bitmap_desc_hash)
-bitmap_desc_hash = htab_create (10, hash_descriptor, eq_descriptor,
NULL);
+ if (!bitmap_desc_hash.is_created ())
+bitmap_desc_hash.create (10);
- slot = (struct bitmap_descriptor **)
-htab_find_slot_with_hash (bitmap_desc_hash, loc,
- htab_hash_pointer (file) + line,
- INSERT);
+ slot = bitmap_desc_hash
+.find_slot_with_hash (loc, htab_hash_pointer (file) + line,
INSERT);
I'd rather split the argument list here.
Done, though it becomes three lines instead of two.
}
-static int
-htab_cu_eq (const void *of1, const void *of2)
+inline bool
+cu_hash_table_entry_hasher::equal (const value_type *entry1,
+ const compare_type *entry2)
No static?
The in-class declaration has the static keyword. (It's still a
global symbol though, not local.)
-static hashval_t
-ttypes_filter_hash (const void *pentry)
+inline hashval_t
+ttypes_filter_hasher::hash (const value_type *entry)
{
- const struct ttypes_filter *entry = (const struct ttypes_filter *)
pentry;
return TREE_HASH (entry-t);
The patch seems to have changed the types here. Is it value_type or
ttypes_filter?
The hasher class defines value_type as a typedef to ttypes_filter.
There has been no change in actual type.
Looks OK otherwise.
Okay, I'll put it in the commit queue.
--
Lawrence Crowl
Re: [cxx-conversion] various hash tables, part 1/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-03 14:24 , Lawrence Crowl wrote: -static int -htab_cu_eq (const void *of1, const void *of2) +inline bool +cu_hash_table_entry_hasher::equal (const value_type *entry1, + const compare_type *entry2) No static? The in-class declaration has the static keyword. (It's still a global symbol though, not local.) Ah, right. I've seen other cases where the patch removes the 'static' qualifier in free functions, though (in the second patch too). Did you need to make those functions externally available? In C++03, any function argument to a template needs to be externally available. These functions are parameters to the traverse templates. -- Lawrence Crowl
Re: [cxx-conversion] various hash tables, part 2/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote:
On 2012-12-01 20:48 , Lawrence Crowl wrote:
+inline bool
+cselib_hasher::equal (const value_type *v, const compare_type *x_arg)
+{
+ struct elt_loc_list *l;
+ rtx x = CONST_CAST_RTX (x_arg);
+ enum machine_mode mode = GET_MODE (x);
+
+ gcc_assert (!CONST_SCALAR_INT_P (x) GET_CODE (x) != CONST_FIXED);
+
+ if (mode != GET_MODE (v-val_rtx))
+return 0;
+
+ /* Unwrap X if necessary. */
+ if (GET_CODE (x) == CONST
+ (CONST_SCALAR_INT_P (XEXP (x, 0))
+ || GET_CODE (XEXP (x, 0)) == CONST_FIXED))
+x = XEXP (x, 0);
+
+ /* We don't guarantee that distinct rtx's have different hash values,
+ so we need to do a comparison. */
+ for (l = v-locs; l; l = l-next)
+if (rtx_equal_for_cselib_1 (l-loc, x, find_slot_memmode))
+ {
+promote_debug_loc (l);
+return 1;
+ }
+
+ return 0;
s/1/true/
s/0/false/
Done.
@@ -504,8 +557,10 @@ cselib_get_next_uid (void)
return next_uid;
}
+#if 0
/* See the documentation of cselib_find_slot below. */
static enum machine_mode find_slot_memmode;
+#endif
Remove?
Done.
+#if 0
/* The equality test for our hash table. The first argument ENTRY is a
table
element (i.e. a cselib_val), while the second arg X is an rtx. We
know
that all callers of htab_find_slot_with_hash will wrap CONST_INTs
into a
@@ -570,6 +626,7 @@ get_value_hash (const void *entry)
const cselib_val *const v = (const cselib_val *) entry;
return v-hash;
}
+#endif
Remove?
Done.
OK otherwise.
This only leaves the GC htabs, right?
No, there are still more non-GC htabs.
What was the issue there? gengtype being silly?
Yes.
--
Lawrence Crowl
Re: [cxx-conversion] ggc-common hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:46 , Lawrence Crowl wrote: Index: gcc/ChangeLog 2012-11-30 Lawrence Crowl cr...@google.com * hash-table.h (class hash_table): Correct many methods with parameter types compare_type to the correct value_type. (Correct code was unlikely to notice the change.) (hash_table::elements_with_deleted) New. ':' after ')'. Done. @@ -1024,11 +1035,10 @@ cmp_statistic (const void *loc1, const v /* Collect array of the descriptors from hashtable. */ static struct loc_descriptor **loc_array; -static int -add_statistics (void **slot, void *b) +int +ggc_add_statistics (loc_descriptor **slot, int *n) Why remove 'static'? It is now a template argument, and cannot be file-local. OK otherwise. Thanks. -- Lawrence Crowl
Re: [cxx-conversion] tree-related hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:45 , Lawrence Crowl wrote: Index: gcc/tree-hasher.h === --- gcc/tree-hasher.h(revision 0) +++ gcc/tree-hasher.h(revision 0) @@ -0,0 +1,56 @@ +/* Data and Control Flow Analysis for Trees. This is the wrong description for this file. + Copyright (C) 2001, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, + 2012 Free Software Foundation, Inc. + Contributed by Diego Novillo dnovi...@redhat.com Only mention year 2012 and don't blame me for sins I never committed ;) The dreaded copy and fail to fully edit. Fixed. OK otherwise. Thanks. -- Lawrence Crowl
Re: [cxx-conversion] gimplify_ctx::temp_htab hash table
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:44 , Lawrence Crowl wrote: Index: gcc/gimple-fold.c === --- gcc/gimple-fold.c(revision 193902) +++ gcc/gimple-fold.c(working copy) @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. #include tree-ssa-propagate.h #include target.h #include gimple-fold.h +#include gimplify-ctx.h /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. Index: gcc/tree-mudflap.c === --- gcc/tree-mudflap.c (revision 193902) +++ gcc/tree-mudflap.c (working copy) @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. #include ggc.h #include cgraph.h #include gimple.h +#include gimplify-ctx.h extern void add_bb_to_loop (basic_block, struct loop *); Index: gcc/tree-inline.c === --- gcc/tree-inline.c(revision 193902) +++ gcc/tree-inline.c(working copy) @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. #include value-prof.h #include tree-pass.h #include target.h +#include gimplify-ctx.h I don't follow. It seems that factoring into gimplify-ctx.h does not actually buy much. The files using it are just including *another* file. Whereas previously, they were getting that content from gimple.h. Unless we can stop including gimple.h from these files, I don't see a lot of gain in this factoring. Am I missing something? There at least 70 files that include gimple.h, and only 5 that need gimple-ctx.h. By splitting it out, at least 65 files will not need to parse the gimplify_ctx struct, the gimple_temp_hash_elt struct, the gimplify_hasher template struct, and may not need to include hash-table.h. It's all about avoiding superfluous compilation in other files. -- Lawrence Crowl
Re: [cxx-conversion] LTO-related hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:40 , Lawrence Crowl wrote: Change LTO-related hash tables from htab_t to hash_table: lto-streamer.h output_block::string_hash_table lto-streamer-in.c file_name_hash_table lto-streamer.c tree_htab The struct string_slot moves from data-streamer.h to lto-streamer.h to resolve compilation dependences. What compilation dependences? If it was fine before in data-streamer.h why does it need to move to lto-streamer.h? I'm missing that connection. Before the change, lto-streamer.h output_block::string_hash_table was an htab_t. The element type was opaque, i.e. implicit in the void* casting in the hash functions provided at the htab_create call site. With the change to hash_table, the output_block::string_hash_table declaration needed string_slot_hasher which in turn needed string_slot. But string_slot was defined in data-streamer.h after output_block in lto-streamer.h. So, something had to move. -- Lawrence Crowl
Re: [cxx-conversion] gimplify_ctx::temp_htab hash table
On 12/4/12, Diego Novillo dnovi...@google.com wrote: On Tue, Dec 4, 2012 at 4:23 AM, Richard Biener richard.guent...@gmail.com wrote: On Mon, Dec 3, 2012 at 9:14 PM, Lawrence Crowl cr...@googlers.com wrote: On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:44 , Lawrence Crowl wrote: Index: gcc/gimple-fold.c === --- gcc/gimple-fold.c(revision 193902) +++ gcc/gimple-fold.c(working copy) @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. #include tree-ssa-propagate.h #include target.h #include gimple-fold.h +#include gimplify-ctx.h /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. Index: gcc/tree-mudflap.c === --- gcc/tree-mudflap.c (revision 193902) +++ gcc/tree-mudflap.c (working copy) @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. #include ggc.h #include cgraph.h #include gimple.h +#include gimplify-ctx.h extern void add_bb_to_loop (basic_block, struct loop *); Index: gcc/tree-inline.c === --- gcc/tree-inline.c(revision 193902) +++ gcc/tree-inline.c(working copy) @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. #include value-prof.h #include tree-pass.h #include target.h +#include gimplify-ctx.h I don't follow. It seems that factoring into gimplify-ctx.h does not actually buy much. The files using it are just including *another* file. Whereas previously, they were getting that content from gimple.h. Unless we can stop including gimple.h from these files, I don't see a lot of gain in this factoring. Am I missing something? There at least 70 files that include gimple.h, and only 5 that need gimple-ctx.h. By splitting it out, at least 65 files will not need to parse the gimplify_ctx struct, the gimple_temp_hash_elt struct, the gimplify_hasher template struct, and may not need to include hash-table.h. It's all about avoiding superfluous compilation in other files. But it's backward - gimple.h is the core file as it provides GIMPLE. The gimplifier and its context certainly requires to know about GIMPLE. Btw, if we still want to split it I'd rather have gimplify.h (and also put all exports from gimplify.c there, not only gimplify_ctx). Still I don't think it would buy us much. We talked offline about this with Lawrence. I was not too convinced about adding this new header file, it seemed forced but I was struggling with a better alternative. Perhaps gimplify.h is a better way, but I'm not sure either. We decided that since it's in the branch, it may not matter much for now. However, another thing occurred to me in the meantime: merges. The new file will probably cause merge problems. If we are not completely convinced that it's a good change, we are making our life harder for no gain. Lawrence, you're going to hate me, but would you mind just leaving the ctx structure in gimple.h for now? Sorry! I think it's going to be easier to just leave it there for now until we come up with a good split for the constructs in that file. Okay, I'll do that. -- Lawrence Crowl
[cxx-conversion] Fix hash_table build problems with checking enabled.
Fix some hash_table build errors when configured with
--enable-checking=yes.
tree-browser.c
* Remove stale declaration of removed TB_parent_eq.
* Fix template parameter for base class to match value_type.
gimple.h
* Use gimplify_hasher::hash rather than gimple_tree_hash in the
assertion check.
* Change return values to match return type. (I.e. no conversions.)
Tested on x86-64.
Committing to branch as obvious.
Index: gcc/tree-browser.c
===
--- gcc/tree-browser.c (revision 194227)
+++ gcc/tree-browser.c (working copy)
@@ -96,14 +96,13 @@ static tree TB_next_expr (tree);
static tree TB_up_expr (tree);
static tree TB_first_in_bind (tree);
static tree TB_last_in_bind (tree);
-static int TB_parent_eq (const void *, const void *);
static tree TB_history_prev (void);
/* FIXME: To be declared in a .h file. */
void browse_tree (tree);
/* Hashtable helpers. */
-struct tree_upper_hasher : typed_noop_remove VALUE
+struct tree_upper_hasher : typed_noop_remove tree_node
{
typedef tree_node value_type;
typedef tree_node compare_type;
Index: gcc/gimple.h
===
--- gcc/gimple.h(revision 194227)
+++ gcc/gimple.h(working copy)
@@ -972,18 +972,18 @@ gimplify_hasher::equal (const value_type
if (TREE_CODE (t2) != code
|| TREE_TYPE (t1) != TREE_TYPE (t2))
-return 0;
+return false;
if (!operand_equal_p (t1, t2, 0))
-return 0;
+return false;
#ifdef ENABLE_CHECKING
/* Only allow them to compare equal if they also hash equal; otherwise
results are nondeterminate, and we fail bootstrap comparison. */
- gcc_assert (gimple_tree_hash (p1) == gimple_tree_hash (p2));
+ gcc_assert (hash (p1) == hash (p2));
#endif
- return 1;
+ return true;
}
struct gimplify_ctx
--
Lawrence Crowl
[cxx-conversion] Convert tree-sra.c'candidates to hash_table
);
-}
-
-/* Hash a tree in a uid_decl_map. */
-
-unsigned int
-uid_decl_map_hash (const void *item)
-{
- return ((const_tree)item)-decl_minimal.uid;
-}
-
-/* Return true if the DECL_UID in both trees are equal. */
-
static int
uid_ssaname_map_eq (const void *va, const void *vb)
{
Index: gcc/tree-flow.h
===
--- gcc/tree-flow.h (revision 194381)
+++ gcc/tree-flow.h (working copy)
@@ -283,9 +283,6 @@ struct int_tree_map {
tree to;
};
-extern unsigned int uid_decl_map_hash (const void *);
-extern int uid_decl_map_eq (const void *, const void *);
-
#define num_ssa_names (vec_safe_length (cfun-gimple_df-ssa_names))
#define ssa_name(i) ((*cfun-gimple_df-ssa_names)[(i)])
Index: gcc/Makefile.in
===
--- gcc/Makefile.in (revision 194381)
+++ gcc/Makefile.in (working copy)
@@ -3034,7 +3034,7 @@ tree-ssa-strlen.o : tree-ssa-strlen.c $(
$(TREE_FLOW_H) $(TREE_PASS_H) domwalk.h alloc-pool.h tree-ssa-propagate.h \
$(GIMPLE_PRETTY_PRINT_H) $(PARAMS_H) $(EXPR_H)
tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \
- $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \
+ $(HASH_TABLE_H) $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \
$(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h \
$(PARAMS_H) $(TARGET_H) $(FLAGS_H) \
$(DBGCNT_H) $(TREE_INLINE_H) $(GIMPLE_PRETTY_PRINT_H)
--
Lawrence Crowl
[cxx-conversion] Convert various htab_t tables to hash_table
())
{
- decl_to_stridxlist_htab
- = htab_create (64, decl_to_stridxlist_hash, tree_map_base_eq, NULL);
+ decl_to_stridxlist_htab.create (64);
gcc_obstack_init (stridx_obstack);
}
ent.base.from = base;
- slot = htab_find_slot_with_hash (decl_to_stridxlist_htab, ent,
- DECL_UID (base), INSERT);
+ slot = decl_to_stridxlist_htab.find_slot_with_hash (ent, DECL_UID (base),
+ INSERT);
if (*slot)
{
int i;
- list = ((struct decl_stridxlist_map *)*slot)-list;
+ list = (*slot)-list;
for (i = 0; i 16; i++)
{
if (list-offset == off)
@@ -273,7 +288,7 @@ addr_stridxptr (tree exp)
struct decl_stridxlist_map *e
= XOBNEW (stridx_obstack, struct decl_stridxlist_map);
e-base.from = base;
- *slot = (void *) e;
+ *slot = e;
list = e-list;
}
list-next = NULL;
@@ -1987,11 +2002,10 @@ tree_ssa_strlen (void)
ssa_ver_to_stridx.release ();
free_alloc_pool (strinfo_pool);
- if (decl_to_stridxlist_htab)
+ if (decl_to_stridxlist_htab.is_created ())
{
obstack_free (stridx_obstack, NULL);
- htab_delete (decl_to_stridxlist_htab);
- decl_to_stridxlist_htab = NULL;
+ decl_to_stridxlist_htab.dispose ();
}
laststmt.stmt = NULL;
laststmt.len = NULL_TREE;
Index: gcc/tree-ssa-loop-ivopts.c
===
--- gcc/tree-ssa-loop-ivopts.c (revision 194381)
+++ gcc/tree-ssa-loop-ivopts.c (working copy)
@@ -76,7 +76,7 @@ along with GCC; see the file COPYING3.
#include ggc.h
#include insn-config.h
#include pointer-set.h
-#include hashtab.h
+#include hash-table.h
#include tree-chrec.h
#include tree-scalar-evolution.h
#include cfgloop.h
@@ -237,6 +237,33 @@ typedef struct iv_use *iv_use_p;
typedef struct iv_cand *iv_cand_p;
+/* Hashtable helpers. */
+
+struct iv_inv_expr_hasher : typed_free_remove iv_inv_expr_ent
+{
+ typedef iv_inv_expr_ent value_type;
+ typedef iv_inv_expr_ent compare_type;
+ static inline hashval_t hash (const value_type *);
+ static inline bool equal (const value_type *, const compare_type *);
+};
+
+/* Hash function for loop invariant expressions. */
+
+inline hashval_t
+iv_inv_expr_hasher::hash (const value_type *expr)
+{
+ return expr-hash;
+}
+
+/* Hash table equality function for expressions. */
+
+inline bool
+iv_inv_expr_hasher::equal (const value_type *expr1, const compare_type *expr2)
+{
+ return expr1-hash == expr2-hash
+ operand_equal_p (expr1-expr, expr2-expr, 0);
+}
+
struct ivopts_data
{
/* The currently optimized loop. */
@@ -256,7 +283,7 @@ struct ivopts_data
/* The hashtable of loop invariant expressions created
by ivopt. */
- htab_t inv_expr_tab;
+ hash_table iv_inv_expr_hasher inv_expr_tab;
/* Loop invariant expression id. */
int inv_expr_id;
@@ -815,30 +842,6 @@ niter_for_single_dom_exit (struct ivopts
return niter_for_exit (data, exit);
}
-/* Hash table equality function for expressions. */
-
-static int
-htab_inv_expr_eq (const void *ent1, const void *ent2)
-{
- const struct iv_inv_expr_ent *expr1 =
- (const struct iv_inv_expr_ent *)ent1;
- const struct iv_inv_expr_ent *expr2 =
- (const struct iv_inv_expr_ent *)ent2;
-
- return expr1-hash == expr2-hash
- operand_equal_p (expr1-expr, expr2-expr, 0);
-}
-
-/* Hash function for loop invariant expressions. */
-
-static hashval_t
-htab_inv_expr_hash (const void *ent)
-{
- const struct iv_inv_expr_ent *expr =
- (const struct iv_inv_expr_ent *)ent;
- return expr-hash;
-}
-
/* Initializes data structures used by the iv optimization pass, stored
in DATA. */
@@ -853,8 +856,7 @@ tree_ssa_iv_optimize_init (struct ivopts
data-niters = NULL;
data-iv_uses.create (20);
data-iv_candidates.create (20);
- data-inv_expr_tab = htab_create (10, htab_inv_expr_hash,
-htab_inv_expr_eq, free);
+ data-inv_expr_tab.create (10);
data-inv_expr_id = 0;
decl_rtl_to_reset.create (20);
}
@@ -3806,8 +3808,7 @@ get_expr_id (struct ivopts_data *data, t
ent.expr = expr;
ent.hash = iterative_hash_expr (expr, 0);
- slot = (struct iv_inv_expr_ent **) htab_find_slot (data-inv_expr_tab,
- ent, INSERT);
+ slot = data-inv_expr_tab.find_slot (ent, INSERT);
if (*slot)
return (*slot)-id;
@@ -6631,7 +6632,7 @@ free_loop_data (struct ivopts_data *data
decl_rtl_to_reset.truncate (0);
- htab_empty (data-inv_expr_tab);
+ data-inv_expr_tab.empty ();
data-inv_expr_id = 0;
}
@@ -6649,7 +6650,7 @@ tree_ssa_iv_optimize_finalize (struct iv
decl_rtl_to_reset.release ();
data-iv_uses.release ();
data-iv_candidates.release ();
- htab_delete (data-inv_expr_tab);
+ data-inv_expr_tab.dispose ();
}
/* Returns true if the loop body BODY includes any function calls. */
--
Lawrence Crowl
[cxx-conversion] Convert tree-vectorizer.h'_loop_vec_info::peeling_htab to hash_table
(LOOP_VINFO_PEELING_HTAB (loop_vinfo), slot,
- INSERT);
+ new_slot = LOOP_VINFO_PEELING_HTAB (loop_vinfo).find_slot (slot, INSERT);
*new_slot = slot;
}
@@ -1330,11 +1307,11 @@ vect_peeling_hash_insert (loop_vec_info
/* Traverse peeling hash table to find peeling option that aligns maximum
number of data accesses. */
-static int
-vect_peeling_hash_get_most_frequent (void **slot, void *data)
+int
+vect_peeling_hash_get_most_frequent (_vect_peel_info **slot,
+_vect_peel_extended_info *max)
{
- vect_peel_info elem = (vect_peel_info) *slot;
- vect_peel_extended_info max = (vect_peel_extended_info) data;
+ vect_peel_info elem = *slot;
if (elem-count max-peel_info.count
|| (elem-count == max-peel_info.count
@@ -1352,11 +1329,11 @@ vect_peeling_hash_get_most_frequent (voi
/* Traverse peeling hash table and calculate cost for each peeling option.
Find the one with the lowest cost. */
-static int
-vect_peeling_hash_get_lowest_cost (void **slot, void *data)
+int
+vect_peeling_hash_get_lowest_cost (_vect_peel_info **slot,
+ _vect_peel_extended_info *min)
{
- vect_peel_info elem = (vect_peel_info) *slot;
- vect_peel_extended_info min = (vect_peel_extended_info) data;
+ vect_peel_info elem = *slot;
int save_misalignment, dummy;
unsigned int inside_cost = 0, outside_cost = 0, i;
gimple stmt = DR_STMT (elem-dr);
@@ -1436,14 +1413,16 @@ vect_peeling_hash_choose_best_peeling (l
{
res.inside_cost = INT_MAX;
res.outside_cost = INT_MAX;
- htab_traverse (LOOP_VINFO_PEELING_HTAB (loop_vinfo),
- vect_peeling_hash_get_lowest_cost, res);
+ LOOP_VINFO_PEELING_HTAB (loop_vinfo)
+ .traverse _vect_peel_extended_info *,
+ vect_peeling_hash_get_lowest_cost (res);
}
else
{
res.peel_info.count = 0;
- htab_traverse (LOOP_VINFO_PEELING_HTAB (loop_vinfo),
- vect_peeling_hash_get_most_frequent, res);
+ LOOP_VINFO_PEELING_HTAB (loop_vinfo)
+ .traverse _vect_peel_extended_info *,
+ vect_peeling_hash_get_most_frequent (res);
}
*npeel = res.peel_info.npeel;
@@ -1648,10 +1627,8 @@ vect_enhance_data_refs_alignment (loop_v
size_zero_node) 0;
/* Save info about DR in the hash table. */
- if (!LOOP_VINFO_PEELING_HTAB (loop_vinfo))
-LOOP_VINFO_PEELING_HTAB (loop_vinfo) =
- htab_create (1, vect_peeling_hash,
-vect_peeling_hash_eq, free);
+ if (!LOOP_VINFO_PEELING_HTAB (loop_vinfo).is_created ())
+LOOP_VINFO_PEELING_HTAB (loop_vinfo).create (1);
vectype = STMT_VINFO_VECTYPE (stmt_info);
nelements = TYPE_VECTOR_SUBPARTS (vectype);
--
Lawrence Crowl
Re: [cxx-conversion] Convert tree-sra.c'candidates to hash_table
On 12/12/12, Diego Novillo dnovi...@google.com wrote:
On Dec 11, 2012 Lawrence Crowl cr...@googlers.com wrote:
Convert tree-sra.c'candidates from htab_t to hash_table.
Fold uid_decl_map_hash and uid_decl_map_eq into new struct
uid_decl_hasher. This change moves the definitions from tree-ssa.c
into tree-sra.c and removes the declarations from tree-flow.h
Update dependent calls and types to hash_table.
Tested on x86_64.
Okay for branch?
Index: gcc/tree-sra.c
+/* Candidate ashtable helpers. */
s/ashtable/hash table/
+struct uid_decl_hasher : typed_noop_remove tree_node
+{
+ typedef tree_node value_type;
+ typedef tree_node compare_type;
+ static inline hashval_t hash (const value_type *);
+ static inline bool equal (const value_type *, const compare_type *);
+};
+
+/* Hash a tree in a uid_decl_map. */
+
+inline hashval_t
+uid_decl_hasher::hash (const value_type *item)
+{
+ return item-decl_minimal.uid;
+}
+
+/* Return true if the DECL_UID in both trees are equal. */
+
+inline bool
+uid_decl_hasher::equal (const value_type *a, const compare_type *b)
+{
+ return (a-decl_minimal.uid == b-decl_minimal.uid);
+}
ISTR other places where we hash decls. I wonder if we shouldn't
move this to some common spot. Maybe later.
Decls are hashed in different ways to match the equality function.
There probably are duplicates in there, but I'm sure they aren't
all duplicates.
The patch is OK.
--
Lawrence Crowl
Re: [cxx-conversion] Convert tree-sra.c'candidates to hash_table
On 12/13/12, Martin Jambor mjam...@suse.cz wrote:
On Thu, Dec 13, 2012 at 11:05:49AM -0800, Lawrence Crowl wrote:
On 12/12/12, Jakub Jelinek ja...@redhat.com wrote:
On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote:
+/* Hash a tree in a uid_decl_map. */
+
+inline hashval_t
+uid_decl_hasher::hash (const value_type *item)
+{
+ return item-decl_minimal.uid;
Ugh, why aren't you using DECL_UID here? The fact that DECL_UID is
right now defined to (DECL_MINIMAL_CHECK (NODE)-decl_minimal.uid)
might change, we really don't want to update thousands of locations
should we need to change that.
That is what the code looked like in its original location. I have
resisted making changes not essential to the purpose of the patch.
However, if you want me to change it to use DECL_UID, I will.
Yes, I think it ought to be changed.
Thanks for the conversion,
It turns out that making the change causes the check to fail.
Therefore, I will leave the code alone and let someone more familiar
with the code investigate.
--
Lawrence Crowl
[cxx-conversion] Convert remaining tree-parloops.c htab_t to hash_table.
(loop_p loop, htab_t reduction_list)
+try_create_reduction_list (loop_p loop,
+ reduction_info_table_type reduction_list)
{
edge exit = single_dom_exit (loop);
gimple_stmt_iterator gsi;
@@ -2036,7 +2054,7 @@ try_create_reduction_list (loop_p loop,
fprintf (dump_file,
checking if it a part of reduction pattern: \n);
}
- if (htab_elements (reduction_list) == 0)
+ if (reduction_list.elements () == 0)
{
if (dump_file (dump_flags TDF_DETAILS))
fprintf (dump_file,
@@ -2110,7 +2128,7 @@ parallelize_loops (void)
struct loop *loop;
struct tree_niter_desc niter_desc;
loop_iterator li;
- htab_t reduction_list;
+ reduction_info_table_type reduction_list;
struct obstack parloop_obstack;
HOST_WIDE_INT estimated;
LOC loop_loc;
@@ -2122,13 +2140,12 @@ parallelize_loops (void)
return false;
gcc_obstack_init (parloop_obstack);
- reduction_list = htab_create (10, reduction_info_hash,
-reduction_info_eq, free);
+ reduction_list.create (10);
init_stmt_vec_info_vec ();
FOR_EACH_LOOP (li, loop, 0)
{
- htab_empty (reduction_list);
+ reduction_list.empty ();
if (dump_file (dump_flags TDF_DETAILS))
{
fprintf (dump_file, Trying loop %d as candidate\n,loop-num);
@@ -2208,7 +2225,7 @@ parallelize_loops (void)
}
free_stmt_vec_info_vec ();
- htab_delete (reduction_list);
+ reduction_list.dispose ();
obstack_free (parloop_obstack, NULL);
/* Parallelization will cause new function calls to be inserted through
--
Lawrence Crowl
[cxx-conversion] Change tree-ssa-coalesce.c'coalesce_list_d.list to hash_table
;
-} coalesce_pair_iterator;
-
-
-/* Return first partition pair from list CL, initializing iterator ITER. */
-
-static inline coalesce_pair_p
-first_coalesce_pair (coalesce_list_p cl, coalesce_pair_iterator *iter)
-{
- coalesce_pair_p pair;
-
- pair = (coalesce_pair_p) first_htab_element ((iter-hti), cl-list);
- return pair;
-}
-
-
-/* Return TRUE if there are no more partitions in for ITER to process. */
-
-static inline bool
-end_coalesce_pair_p (coalesce_pair_iterator *iter)
-{
- return end_htab_p ((iter-hti));
-}
-
-
-/* Return the next partition pair to be visited by ITER. */
-
-static inline coalesce_pair_p
-next_coalesce_pair (coalesce_pair_iterator *iter)
-{
- coalesce_pair_p pair;
-
- pair = (coalesce_pair_p) next_htab_element ((iter-hti));
- return pair;
+ return cl-list.elements ();
}
-/* Iterate over CL using ITER, returning values in PAIR. */
-
-#define FOR_EACH_PARTITION_PAIR(PAIR, ITER, CL)\
- for ((PAIR) = first_coalesce_pair ((CL), (ITER)); \
- !end_coalesce_pair_p ((ITER)); \
- (PAIR) = next_coalesce_pair ((ITER)))
-
-
/* Prepare CL for removal of preferred pairs. When finished they are sorted
in order from most important coalesce to least important. */
@@ -416,7 +373,7 @@ sort_coalesce_list (coalesce_list_p cl)
{
unsigned x, num;
coalesce_pair_p p;
- coalesce_pair_iterator ppi;
+ coalesce_iterator_type ppi;
gcc_assert (cl-sorted == NULL);
@@ -430,7 +387,7 @@ sort_coalesce_list (coalesce_list_p cl)
/* Populate the vector with pointers to the pairs. */
x = 0;
- FOR_EACH_PARTITION_PAIR (p, ppi, cl)
+ FOR_EACH_HASH_TABLE_ELEMENT (cl-list, p, coalesce_pair_p, ppi)
cl-sorted[x++] = p;
gcc_assert (x == num);
@@ -462,14 +419,15 @@ static void
dump_coalesce_list (FILE *f, coalesce_list_p cl)
{
coalesce_pair_p node;
- coalesce_pair_iterator ppi;
+ coalesce_iterator_type ppi;
+
int x;
tree var;
if (cl-sorted == NULL)
{
fprintf (f, Coalesce List:\n);
- FOR_EACH_PARTITION_PAIR (node, ppi, cl)
+ FOR_EACH_HASH_TABLE_ELEMENT (cl-list, node, coalesce_pair_p, ppi)
{
tree var1 = ssa_name (node-first_element);
tree var2 = ssa_name (node-second_element);
--
Lawrence Crowl
Re: [cxx-conversion] Change tree-ssa-coalesce.c'coalesce_list_d.list to hash_table
On 12/17/12, Richard Biener richard.guent...@gmail.com wrote:
On Mon, Dec 17, 2012 at 8:36 PM, Lawrence Crowl cr...@googlers.com wrote:
Change tree-ssa-coalesce.c'coalesce_list_d.list from htab_t to
hash_table.
Fold coalesce_pair_map_hash and coalesce_pair_map_eq into new
struct coalesce_pair_hasher.
Removed struct coalesce_pair_iterator, as did not meet the hash_table
iterator interface and it provided no significant code reduction.
Tested on x86-64.
Okay for branch?
Index: gcc/tree-ssa-coalesce.c
===
--- gcc/tree-ssa-coalesce.c (revision 194549)
+++ gcc/tree-ssa-coalesce.c (working copy)
@@ -50,6 +50,41 @@ typedef struct coalesce_pair
} * coalesce_pair_p;
typedef const struct coalesce_pair *const_coalesce_pair_p;
+/* Coalesce pair hashtable helpers. */
+
+struct coalesce_pair_hasher : typed_noop_remove coalesce_pair
+{
+ typedef coalesce_pair value_type;
+ typedef coalesce_pair compare_type;
+ static inline hashval_t hash (const value_type *);
+ static inline bool equal (const value_type *, const compare_type *);
+};
+
+/* Hash function for coalesce list. Calculate hash for PAIR. */
+
+inline hashval_t
+coalesce_pair_hasher::hash (const value_type *pair)
+{
+ hashval_t a = (hashval_t)(pair-first_element);
+ hashval_t b = (hashval_t)(pair-second_element);
+
+ return b * (b - 1) / 2 + a;
+}
+
+/* Equality function for coalesce list hash table. Compare PAIR1 and
PAIR2,
+ returning TRUE if the two pairs are equivalent. */
+
+inline bool
+coalesce_pair_hasher::equal (const value_type *p1, const compare_type
*p2)
+{
+ return (p1-first_element == p2-first_element
+ p1-second_element == p2-second_element);
+}
+
+typedef hash_table coalesce_pair_hasher coalesce_table_type;
+typedef coalesce_table_type::iterator coalesce_iterator_type;
+
+
typedef struct cost_one_pair_d
{
int first_element;
@@ -61,7 +96,7 @@ typedef struct cost_one_pair_d
typedef struct coalesce_list_d
{
- htab_t list; /* Hash table. */
+ coalesce_table_type list;/* Hash table. */
coalesce_pair_p *sorted; /* List when sorted. */
int num_sorted; /* Number in the sorted list. */
cost_one_pair_p cost_one_list;/* Single use coalesces with cost 1. */
@@ -186,34 +221,6 @@ pop_best_coalesce (coalesce_list_p cl, i
}
-#define COALESCE_HASH_FN(R1, R2) ((R2) * ((R2) - 1) / 2 + (R1))
-
-/* Hash function for coalesce list. Calculate hash for PAIR. */
-
-static unsigned int
-coalesce_pair_map_hash (const void *pair)
-{
- hashval_t a =
(hashval_t)(((const_coalesce_pair_p)pair)-first_element);
- hashval_t b =
(hashval_t)(((const_coalesce_pair_p)pair)-second_element);
-
- return COALESCE_HASH_FN (a,b);
-}
-
-
-/* Equality function for coalesce list hash table. Compare PAIR1 and
PAIR2,
- returning TRUE if the two pairs are equivalent. */
-
-static int
-coalesce_pair_map_eq (const void *pair1, const void *pair2)
-{
- const_coalesce_pair_p const p1 = (const_coalesce_pair_p) pair1;
- const_coalesce_pair_p const p2 = (const_coalesce_pair_p) pair2;
-
- return (p1-first_element == p2-first_element
- p1-second_element == p2-second_element);
-}
-
-
/* Create a new empty coalesce list object and return it. */
static inline coalesce_list_p
@@ -226,8 +233,7 @@ create_coalesce_list (void)
size = 40;
list = (coalesce_list_p) xmalloc (sizeof (struct coalesce_list_d));
- list-list = htab_create (size, coalesce_pair_map_hash,
- coalesce_pair_map_eq, NULL);
+ list-list.create (size);
list-sorted = NULL;
list-num_sorted = 0;
list-cost_one_list = NULL;
@@ -241,7 +247,7 @@ static inline void
delete_coalesce_list (coalesce_list_p cl)
{
gcc_assert (cl-cost_one_list == NULL);
- htab_delete (cl-list);
+ cl-list.dispose ();
free (cl-sorted);
gcc_assert (cl-num_sorted == 0);
free (cl);
@@ -256,7 +262,7 @@ static coalesce_pair_p
find_coalesce_pair (coalesce_list_p cl, int p1, int p2, bool create)
{
struct coalesce_pair p;
- void **slot;
+ coalesce_pair **slot;
unsigned int hash;
/* Normalize so that p1 is the smaller value. */
@@ -271,9 +277,8 @@ find_coalesce_pair (coalesce_list_p cl,
p.second_element = p2;
}
- hash = coalesce_pair_map_hash (p);
- slot = htab_find_slot_with_hash (cl-list, p, hash,
- create ? INSERT : NO_INSERT);
+ hash = coalesce_pair_hasher::hash (p);
+ slot = cl-list.find_slot_with_hash (p, hash, create ? INSERT :
NO_INSERT);
if (!slot)
return NULL;
@@ -284,7 +289,7 @@ find_coalesce_pair (coalesce_list_p cl,
pair-first_element = p.first_element;
pair-second_element = p.second_element;
pair-cost = 0;
- *slot = (void *)pair;
+ *slot = pair;
}
return (struct
Re: [PATCH] Fix up sbitmap bitmap_{and,ior,xor} (PR target/55562)
On 12/18/12, Jakub Jelinek ja...@redhat.com wrote:
The bitmap unification changes apparently broke at least modulo
scheduling, which used sbitmap_a_and_b_cg functions, which
were replaced by bitmap_and. But, sbitmap_a_and_b_cg asserted
dst-popcount is NULL and returned whether dst sbitmap changed,
but bitmap_and returns always false if dst-popcount is NULL
(and only if it is non-NULL returns if the bitmap changed).
Fixed thusly, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?
The changes look okay to me. (But I don't approve trunk.)
Alternatively we could add back separate functions that would
return whether dest changed for speed reasons (similarly to
the old *_cg ones), and return void from the normal ones again.
I bet most of the users don't check the return value of these
functions and thus it is useless computation.
The discussion at the time was pretty conclusive to not have
separate functions.
2012-12-18 Jakub Jelinek ja...@redhat.com
PR target/55562
* sbitmap.c (bitmap_and, bitmap_xor, bitmap_ior): Return whether
dst sbitmap changed even if it doesn't have popcount.
--- gcc/sbitmap.c.jj 2012-11-02 09:01:54.0 +0100
+++ gcc/sbitmap.c 2012-12-18 14:29:13.695299294 +0100
@@ -434,28 +434,26 @@ bitmap_and (sbitmap dst, const_sbitmap a
const_sbitmap_ptr bp = b-elms;
bool has_popcount = dst-popcount != NULL;
unsigned char *popcountp = dst-popcount;
- bool anychange = false;
+ SBITMAP_ELT_TYPE changed = 0;
for (i = 0; i n; i++)
{
const SBITMAP_ELT_TYPE tmp = *ap++ *bp++;
+ SBITMAP_ELT_TYPE wordchanged = *dstp ^ tmp;
if (has_popcount)
{
- bool wordchanged = (*dstp ^ tmp) != 0;
if (wordchanged)
- {
- *popcountp = do_popcount (tmp);
- anychange = true;
- }
+ *popcountp = do_popcount (tmp);
popcountp++;
}
*dstp++ = tmp;
+ changed |= wordchanged;
}
#ifdef BITMAP_DEBUGGING
if (has_popcount)
sbitmap_verify_popcount (dst);
#endif
- return anychange;
+ return changed != 0;
}
/* Set DST to be (A xor B)).
@@ -470,28 +468,26 @@ bitmap_xor (sbitmap dst, const_sbitmap a
const_sbitmap_ptr bp = b-elms;
bool has_popcount = dst-popcount != NULL;
unsigned char *popcountp = dst-popcount;
- bool anychange = false;
+ SBITMAP_ELT_TYPE changed = 0;
for (i = 0; i n; i++)
{
const SBITMAP_ELT_TYPE tmp = *ap++ ^ *bp++;
+ SBITMAP_ELT_TYPE wordchanged = *dstp ^ tmp;
if (has_popcount)
{
- bool wordchanged = (*dstp ^ tmp) != 0;
if (wordchanged)
- {
- *popcountp = do_popcount (tmp);
- anychange = true;
- }
+ *popcountp = do_popcount (tmp);
popcountp++;
}
*dstp++ = tmp;
+ changed |= wordchanged;
}
#ifdef BITMAP_DEBUGGING
if (has_popcount)
sbitmap_verify_popcount (dst);
#endif
- return anychange;
+ return changed != 0;
}
/* Set DST to be (A or B)).
@@ -506,28 +502,26 @@ bitmap_ior (sbitmap dst, const_sbitmap a
const_sbitmap_ptr bp = b-elms;
bool has_popcount = dst-popcount != NULL;
unsigned char *popcountp = dst-popcount;
- bool anychange = false;
+ SBITMAP_ELT_TYPE changed = 0;
for (i = 0; i n; i++)
{
const SBITMAP_ELT_TYPE tmp = *ap++ | *bp++;
+ SBITMAP_ELT_TYPE wordchanged = *dstp ^ tmp;
if (has_popcount)
{
- bool wordchanged = (*dstp ^ tmp) != 0;
if (wordchanged)
- {
- *popcountp = do_popcount (tmp);
- anychange = true;
- }
+ *popcountp = do_popcount (tmp);
popcountp++;
}
*dstp++ = tmp;
+ changed |= wordchanged;
}
#ifdef BITMAP_DEBUGGING
if (has_popcount)
sbitmap_verify_popcount (dst);
#endif
- return anychange;
+ return changed != 0;
}
/* Return nonzero if A is a subset of B. */
--
Lawrence Crowl
[cxx-conversion] Change uses of htab_t in gcc/config to hash_table.
*);
+};
+
/* Hash-table callbacks for mips_lo_sum_offsets. */
-static hashval_t
-mips_lo_sum_offset_hash (const void *entry)
+inline hashval_t
+mips_lo_sum_offset_hasher::hash (const value_type *entry)
{
- return mips_hash_base (((const struct mips_lo_sum_offset *) entry)-base);
+ return mips_hash_base (entry-base);
}
-static int
-mips_lo_sum_offset_eq (const void *entry, const void *value)
+inline bool
+mips_lo_sum_offset_hasher::equal (const value_type *entry,
+ const compare_type *value)
{
- return rtx_equal_p (((const struct mips_lo_sum_offset *) entry)-base,
- (const_rtx) value);
+ return rtx_equal_p (entry-base, value);
}
+typedef hash_table mips_lo_sum_offset_hasher mips_offset_table;
+
/* Look up symbolic constant X in HTAB, which is a hash table of
mips_lo_sum_offsets. If OPTION is NO_INSERT, return true if X can be
paired with a recorded LO_SUM, otherwise record X in the table. */
static bool
-mips_lo_sum_offset_lookup (htab_t htab, rtx x, enum insert_option option)
+mips_lo_sum_offset_lookup (mips_offset_table htab, rtx x,
+ enum insert_option option)
{
rtx base, offset;
- void **slot;
+ mips_lo_sum_offset **slot;
struct mips_lo_sum_offset *entry;
/* Split X into a base and offset. */
@@ -15624,7 +15637,7 @@ mips_lo_sum_offset_lookup (htab_t htab,
base = UNSPEC_ADDRESS (base);
/* Look up the base in the hash table. */
- slot = htab_find_slot_with_hash (htab, base, mips_hash_base (base), option);
+ slot = htab.find_slot_with_hash (base, mips_hash_base (base), option);
if (slot == NULL)
return false;
@@ -15654,7 +15667,8 @@ static int
mips_record_lo_sum (rtx *loc, void *data)
{
if (GET_CODE (*loc) == LO_SUM)
-mips_lo_sum_offset_lookup ((htab_t) data, XEXP (*loc, 1), INSERT);
+mips_lo_sum_offset_lookup (*(mips_offset_table*)data,
+ XEXP (*loc, 1), INSERT);
return 0;
}
@@ -15663,7 +15677,7 @@ mips_record_lo_sum (rtx *loc, void *data
LO_SUMs in the current function. */
static bool
-mips_orphaned_high_part_p (htab_t htab, rtx insn)
+mips_orphaned_high_part_p (mips_offset_table htab, rtx insn)
{
enum mips_symbol_type type;
rtx x, set;
@@ -15771,7 +15785,7 @@ mips_reorg_process_insns (void)
{
rtx insn, last_insn, subinsn, next_insn, lo_reg, delayed_reg;
int hilo_delay;
- htab_t htab;
+ mips_offset_table htab;
/* Force all instructions to be split into their final form. */
split_all_insns_noflow ();
@@ -15808,14 +15822,13 @@ mips_reorg_process_insns (void)
if (TARGET_FIX_VR4130 !ISA_HAS_MACCHI)
cfun-machine-all_noreorder_p = false;
- htab = htab_create (37, mips_lo_sum_offset_hash,
- mips_lo_sum_offset_eq, free);
+ htab.create (37);
/* Make a first pass over the instructions, recording all the LO_SUMs. */
for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
FOR_EACH_SUBINSN (subinsn, insn)
if (USEFUL_INSN_P (subinsn))
- for_each_rtx (PATTERN (subinsn), mips_record_lo_sum, htab);
+ for_each_rtx (PATTERN (subinsn), mips_record_lo_sum, htab);
last_insn = 0;
hilo_delay = 2;
@@ -15872,7 +15885,7 @@ mips_reorg_process_insns (void)
}
}
- htab_delete (htab);
+ htab.dispose ();
}
/* Return true if the function has a long branch instruction. */
--
Lawrence Crowl
Re: [patch] std::unique_ptrT[], D improvements
On 12/20/12, Jonathan Wakely jwakely@gmail.com wrote: This patch started when I noticed that it's not possibly to construct a shared_ptrT from unique_ptrT[], D, then I discovered we don't use D::pointer if it exists, and there were a number of other non-conformance issues with our std::unique_ptrT[], D. I ended up fixing them by implementing Geoffrey's proposed resolution for LWG issue 2118, which isn't official yet but is better than what we had before so is a step in the right direction, even if it ends up needing further revision when 2118 is resolved. * include/std/functional (_Require): Move to ... * include/std/type_traits (_Require): ... here. * include/bits/shared_ptr_base.h (__shared_count::_S_create_from_up): Handle unique_ptr for arrays or with custom pointer types. (__shared_ptr::__shared_ptr(unique_ptr_Tp1, _Del): Likewise. * include/bits/unique_ptr.h (unique_ptr_Tp[], _Dp): Use _Dp::pointer if defined. Implement proposed resolution of LWG 2118. * testsuite/20_util/shared_ptr/cons/unique_ptr_array.cc: New. * testsuite/20_util/unique_ptr/assign/cv_qual.cc: New. * testsuite/20_util/unique_ptr/cons/array_convertible_neg.cc: New. * testsuite/20_util/unique_ptr/cons/convertible_neg.cc: New. * testsuite/20_util/unique_ptr/cons/cv_qual.cc: New. * testsuite/20_util/unique_ptr/modifiers/cv_qual.cc: New. * testsuite/20_util/unique_ptr/requirements/pointer_type_array.cc: New. * testsuite/20_util/shared_ptr/cons/unique_ptr.cc: Adjust comments. * testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc: Likewise. * testsuite/20_util/unique_ptr/requirements/pointer_type.cc: Likewise. * testsuite/20_util/bind/ref_neg.cc: Adjust dg-error line number. * testsuite/20_util/declval/requirements/1_neg.cc: Likewise. * testsuite/20_util/default_delete/48631_neg.cc: Likewise. * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Likewise. * testsuite/20_util/unique_ptr/assign/48635_neg.cc: Likewise. * testsuite/20_util/unique_ptr/modifiers/reset_neg.cc: Adjust dg-error text. * testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc: Use different instantiations so static_assert fails for each. Thanks to Geoffrey and Lawrence for input and test cases. Tested x86_64-linux, committed to trunk. I'm not getting errors when converting from derived to base. E.g. the following compiles, when it should not. std::unique_ptrconst base [] acb_ad(new derived[3]); -- Lawrence Crowl
Re: [patch] std::unique_ptrT[], D improvements
On 12/28/12, Jonathan Wakely jwakely@gmail.com wrote:
On 28 December 2012 01:51, Lawrence Crowl wrote:
I'm not getting errors when converting from derived to base.
E.g. the following compiles, when it should not.
std::unique_ptrconst base [] acb_ad(new derived[3]);
I get an error:
shm$ cat up.cc
#include memory
struct base { };
struct derived : base { virtual ~derived() = default; };
std::unique_ptrconst base [] acb_ad(new derived[3]);
shm$
shm$ g++11 up.cc -c
up.cc:4:53: error: use of deleted function ‘std::unique_ptr_Tp [],
_Dp::unique_ptr(_Up*) [with _Up = derived; template-parameter-2-2 =
void; _Tp = const base; _Dp = std::default_deleteconst base []]’
std::unique_ptrconst base [] acb_ad(new derived[3]);
^
In file included from /home/redi/gcc/4.x/include/c++/4.8.0/memory:81:0,
from up.cc:1:
/home/redi/gcc/4.x/include/c++/4.8.0/bits/unique_ptr.h:343:2: error:
declared here
unique_ptr(_Up* __p) = delete;
^
That was pilot error on my part. However, I've been having trouble
when the argument to the constructor or reset has a conversion
operator. The code does distinquish between a safe conversion to
base and an unsafe conversion to derived.
Here is a simplified version of the problem. The code as is fails
to reject the last two calls to accept. The primary reason is that
is_convertable permits both the invocation of the conversion operator
and the derived to base conversion. At present, I see no way to
get a handle on the 'natural' return type of the conversion operator.
#include type_traits
struct base { };
struct derived : base { };
template typename T, typename F
typename std::enable_if
std::is_convertible F, T* ::value,
T*
::type
accept(F arg) { return arg; }
template typename T, typename F
typename std::enable_if
!std::is_convertible F(*)[], T(*)[] ::value,
T*
::type
accept(F* arg) = delete;
struct cvt_b {
operator base*() { return 0; }
};
struct cvt_d {
operator derived*() { return 0; }
};
int main()
{
// should PASS
accept base ( (base*)0 );
accept const base ( (base*)0 );
accept base ( cvt_b() );
accept const base ( cvt_b() );
// should FAIL
accept base ( (derived*)0 );
accept const base ( (derived*)0 );
accept base ( cvt_d() );
accept const base ( cvt_d() );
}
--
Lawrence Crowl
Re: [patch] std::unique_ptrT[], D improvements
On 1/1/13, Jonathan Wakely jwakely@gmail.com wrote: On 1 January 2013 20:40, Lawrence Crowl wrote: That was pilot error on my part. However, I've been having trouble when the argument to the constructor or reset has a conversion operator. The code does distinquish between a safe conversion to base and an unsafe conversion to derived. Here is a simplified version of the problem. The code as is fails to reject the last two calls to accept. The primary reason is that is_convertable permits both the invocation of the conversion operator and the derived to base conversion. At present, I see no way to get a handle on the 'natural' return type of the conversion operator. Is the issue here that Geoffrey's proposed resolution allows any conversions (safe or not) if the source type is not a built-in pointer, i.e. is some user-defined type? So a user-defined type that refers an array of derived classes is allowed to be converted to an array of base, even though that's not safe. Howard has strongly objected to this part of the P/R. Yes. I see no way to distinguish between objects with safe conversion operators and unsafe conversion operators. -- Lawrence Crowl
[google 4_7] backport std::unique_ptrT[], D improvements
]
#include memory
-#include testsuite_hooks.h
using std::unique_ptr;
@@ -30,9 +29,9 @@ using std::unique_ptr;
void
test01()
{
- unique_ptrint, void(*)(int*) p1; // { dg-error here }
+ unique_ptrlong, void(*)(long*) p1; // { dg-error here }
- unique_ptrint, void(*)(int*) p2(nullptr); // { dg-error here }
+ unique_ptrshort, void(*)(short*) p2(nullptr); // { dg-error here }
unique_ptrint, void(*)(int*) p3(new int); // { dg-error here }
}
@@ -40,9 +39,9 @@ test01()
void
test02()
{
- unique_ptrint[], void(*)(int*) p1; // { dg-error here }
+ unique_ptrlong[], void(*)(long*) p1; // { dg-error here }
- unique_ptrint[], void(*)(int*) p2(nullptr); // { dg-error here }
+ unique_ptrshort[], void(*)(short*) p2(nullptr); // { dg-error here }
unique_ptrint[], void(*)(int*) p3(new int[1]); // { dg-error here }
}
Index:
libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc
===
---
libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc
(revision
194742)
+++
libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc
(working
copy)
@@ -1,7 +1,7 @@
// { dg-do compile }
-// { dg-options -std=gnu++0x }
+// { dg-options -std=gnu++11 }
-// Copyright (C) 2008, 2009 Free Software Foundation
+// Copyright (C) 2008-2012 Free Software Foundation
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -29,7 +29,7 @@ struct B : A
virtual ~B() { }
};
-// 20.4.5.1 unique_ptr constructors [unique.ptr.cons]
+// 20.7.1.3.1 unique_ptr constructors [unique.ptr.runtime.ctor]
// Construction from pointer of derived type
void
Index: libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc
===
--- libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc
(revision
194742)
+++ libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc
(working
copy)
@@ -1,7 +1,7 @@
// { dg-do compile }
-// { dg-options -std=gnu++0x }
+// { dg-options -std=gnu++11 }
-// Copyright (C) 2010, 2011 Free Software Foundation
+// Copyright (C) 2010-2012 Free Software Foundation
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -18,10 +18,9 @@
// with this library; see the file COPYING3. If not see
// http://www.gnu.org/licenses/.
-// 20.6.11 Template class unique_ptr [unique.ptr.single]
+// 20.7.1.2 unique_ptr for single objects [unique.ptr.single]
#include memory
-#include testsuite_hooks.h
struct A
{
Index: libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
===
--- libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
(revision
194742)
+++ libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
(working copy)
@@ -41,10 +41,10 @@ void f()
std::unique_ptrint, B ub(nullptr, b);
std::unique_ptrint, D ud(nullptr, d);
ub = std::move(ud);
-// { dg-error use of deleted function { target *-*-* } 195 }
+// { dg-error use of deleted function { target *-*-* } 203 }
std::unique_ptrint[], B uba(nullptr, b);
std::unique_ptrint[], D uda(nullptr, d);
uba = std::move(uda);
-// { dg-error use of deleted function { target *-*-* } 341 }
+// { dg-error use of deleted function { target *-*-* } 393 }
}
Index: libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc
===
--- libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc (revision
194742)
+++ libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc (working copy)
@@ -27,4 +27,4 @@ struct D : B { };
D d;
std::default_deleteB[] db;
typedef decltype(db(d)) type; // { dg-error use of deleted function }
-// { dg-error declared here { target *-*-* } 83 }
+// { dg-error declared here { target *-*-* } 100 }
Index: libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc
===
--- libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc
(revision
194742)
+++ libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc
(working copy)
@@ -19,7 +19,7 @@
// with this library; see the file COPYING3. If not see
// http://www.gnu.org/licenses/.
-// { dg-error static assertion failed { target *-*-* } 1776 }
+// { dg-error static assertion failed { target *-*-* } 1778 }
#include utility
--
Lawrence Crowl
Re: [patch] std::unique_ptrT[], D improvements
On 1/1/13, Geoffrey Romer gro...@google.com wrote: AFAICT there's no way to distinguish between safe and unsafe conversions of user-defined pointers, because that's a property of the pointer implementation, not the type itself. My PR errs on the side of trusting the implementation to provide only correct conversions. As Jonathan notes, Howard has objected to that part of the PR, so it's possible the eventual resolution will differ in that respect; I intend to pick up that discussion next week when I'm back from vacation. BTW, I've attached my latest set of tests. -- Lawrence Crowl tests.tar.gz Description: GNU Zip compressed data
Re: [cxx-conversion] Change uses of htab_t in gcc/config to hash_table.
On 1/6/13, Richard Biener richard.guent...@gmail.com wrote:
On Sun, Jan 6, 2013 at 6:55 AM, Xinliang David Li davi...@google.com wrote:
I noticed that the traverse and traverse_noresize method takes
Argument as the first template parameter. It should be moved to be the
second after the callback. In most of the cases, the type of the
Argument can be deduced from the callsite, so that the user only need
to specify the callback:
ht-traverse_noresizethe_call_back_function(arg);
In the current way, user has to do this:
ht-traverse_noresizearg_type, the_call_back_function (arg);
which is not as friendly.
Agreed.
Agreed. The current structure was handy in the conversion process.
Now that the conversion is done, we could change the order.
David
On Tue, Dec 18, 2012 at 8:02 PM, Lawrence Crowl cr...@googlers.com
wrote:
Update various config htab_t uses to hash_table.
Modify types and calls to match.
* config/arm/arm.c'arm_libcall_uses_aapcs_base::libcall_htab
Fold libcall_eq and libcall_hash into new struct libcall_hasher.
* config/ia64/ia64.c'bundle_state_table
Fold bundle_state_hash and bundle_state_eq_p into new struct
bundle_state_hasher.
* config/mips/mips.c'mips_offset_table
Fold mips_lo_sum_offset_hash and mips_lo_sum_offset_eq into new
struct mips_lo_sum_offset_hasher.
In mips_reorg_process_insns, change call to for_each_rtx to pass
a pointer to the hash_table rather than a htab_t. This change
requires then dereferencing that pointer in mips_record_lo_sum to
obtain the hash_table.
* config/sol2.c'solaris_comdat_htab
Fold comdat_hash and comdat_eq into new struct comdat_entry_hasher.
* config/i386/winnt.c'i386_pe_section_type_flags::htab
* config/i386/winnt.c'i386_find_on_wrapper_list::wrappers
Fold wrapper_strcmp into new struct wrapped_symbol_hasher.
Tested on x86-64.
Tested with contrib/config-list.mk.
Okay for branch?
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 194511)
+++ gcc/config/arm/arm.c(working copy)
@@ -25,6 +25,7 @@
#include config.h
#include system.h
#include coretypes.h
+#include hash-table.h
#include tm.h
#include rtl.h
#include tree.h
@@ -3716,36 +3717,48 @@ arm_function_value(const_tree type, cons
return arm_libcall_value_1 (mode);
}
-static int
-libcall_eq (const void *p1, const void *p2)
+/* libcall hashtable helpers. */
+
+struct libcall_hasher : typed_noop_remove rtx_def
{
- return rtx_equal_p ((const_rtx) p1, (const_rtx) p2);
+ typedef rtx_def value_type;
+ typedef rtx_def compare_type;
+ static inline hashval_t hash (const value_type *);
+ static inline bool equal (const value_type *, const compare_type *);
+ static inline void remove (value_type *);
+};
+
+inline bool
+libcall_hasher::equal (const value_type *p1, const compare_type *p2)
+{
+ return rtx_equal_p (p1, p2);
}
-static hashval_t
-libcall_hash (const void *p1)
+inline hashval_t
+libcall_hasher::hash (const value_type *p1)
{
- return hash_rtx ((const_rtx) p1, VOIDmode, NULL, NULL, FALSE);
+ return hash_rtx (p1, VOIDmode, NULL, NULL, FALSE);
}
+typedef hash_table libcall_hasher libcall_table_type;
+
static void
-add_libcall (htab_t htab, rtx libcall)
+add_libcall (libcall_table_type htab, rtx libcall)
{
- *htab_find_slot (htab, libcall, INSERT) = libcall;
+ *htab.find_slot (libcall, INSERT) = libcall;
}
static bool
arm_libcall_uses_aapcs_base (const_rtx libcall)
{
static bool init_done = false;
- static htab_t libcall_htab;
+ static libcall_table_type libcall_htab;
if (!init_done)
{
init_done = true;
- libcall_htab = htab_create (31, libcall_hash, libcall_eq,
- NULL);
+ libcall_htab.create (31);
add_libcall (libcall_htab,
convert_optab_libfunc (sfloat_optab, SFmode,
SImode));
add_libcall (libcall_htab,
@@ -3804,7 +3817,7 @@ arm_libcall_uses_aapcs_base (const_rtx l
DFmode));
}
- return libcall htab_find (libcall_htab, libcall) != NULL;
+ return libcall libcall_htab.find (libcall) != NULL;
}
static rtx
Index: gcc/config/arm/t-arm
===
--- gcc/config/arm/t-arm(revision 194511)
+++ gcc/config/arm/t-arm(working copy)
@@ -73,8 +73,8 @@ $(srcdir)/config/arm/arm-tables.opt: $(s
$(SHELL) $(srcdir)/config/arm/genopt.sh $(srcdir)/config/arm \
$(srcdir)/config/arm/arm-tables.opt
-arm.o: $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
- $(RTL_H) $(TREE_H) $(OBSTACK_H) $(REGS_H) hard-reg-set.h \
+arm.o: $(srcdir)/config/arm/arm.c $(CONFIG_H) $(SYSTEM_H) coretypes.h
$(TM_H) \
+ $(RTL_H) $(TREE_H) $(HASH_TABLE_H) $(OBSTACK_H) $(REGS_H)
hard-reg-set.h \
insn-config.h conditions.h
Re: [PATCH 1/2] [asan] Allow creating/deleting hash table entries with new/delete
On 1/28/13, Dodji Seketeli do...@redhat.com wrote:
Hello,
The hash table type can handle creation and removal of entries with
malloc/free. This patchlet adds support for using new/delete. It's
useful for hash table entry types that have constructors (and/or
destructors), to prevent the user from having to type boilerplate code
to initialize them over and over again. This is used by the patch that
follows this one.
Looks good to me.
gcc/
* hash-table.h (struct typed_delete_remove): New type.
(typed_delete_remove::remove): Implement this using the delete
operator.
---
gcc/hash-table.h | 16
1 file changed, 16 insertions(+)
diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 206423d..884840c 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -235,6 +235,22 @@ typed_free_remove Type::remove (Type *p)
free (p);
}
+/* Helpful type for removing entries with the delete operator. */
+
+template typename Type
+struct typed_delete_remove
+{
+ static inline void remove (Type *p);
+};
+
+/* Remove with delete. */
+
+template typename Type
+inline void
+typed_delete_remove Type::remove (Type *p)
+{
+ delete p;
+}
/* Helpful type for a no-op remove. */
--
1.7.11.7
--
Dodji
--
Lawrence Crowl
[cxx-conversion] Add Record Builder Class
(BUILTINS_LOCATION, FIELD_DECL,
- get_identifier (offset), unsigned_type_node);
- DECL_CONTEXT (field) = type;
- DECL_CHAIN (field) = next_field;
-
- return field;
+ record_builder rec;
+ rec.add_field (offset, unsigned_type_node);
+ rec.add_field (module_id, unsigned_type_node);
+ rec.add_field (size, unsigned_type_node);
+ rec.layout ();
+ rec.decl_name (__tls_var);
+ return rec.as_tree ();
}
/* Return the CONSTRUCTOR to initialize an emulated TLS control
@@ -131,7 +114,7 @@ vxworks_override_options (void)
targetm.emutls.tmpl_section = .tls_data;
targetm.emutls.var_prefix = __tls__;
targetm.emutls.tmpl_prefix = ;
- targetm.emutls.var_fields = vxworks_emutls_var_fields;
+ targetm.emutls.object_type = vxworks_emutls_object_type;
targetm.emutls.var_init = vxworks_emutls_var_init;
targetm.emutls.var_align_fixed = true;
targetm.emutls.debug_form_tls_address = true;
Index: gcc/target.def
===
--- gcc/target.def (revision 195904)
+++ gcc/target.def (working copy)
@@ -2741,12 +2741,12 @@ DEFHOOKPOD
,
const char *, NULL)
-/* Function to generate field definitions of the proxy variable. */
+/* Function to generate type of the proxy variable. */
DEFHOOK
-(var_fields,
+(object_type,
,
- tree, (tree type, tree *name),
- default_emutls_var_fields)
+ tree, (),
+ default_emutls_object_type)
/* Function to initialize a proxy variable. */
DEFHOOK
Index: gcc/tree.c
===
--- gcc/tree.c (revision 195904)
+++ gcc/tree.c (working copy)
@@ -11624,4 +11624,95 @@ warn_deprecated_use (tree node, tree att
}
}
+
+/* Construct a record builder with the identifier IDENT.
+ It is a union if IS_UNION is true, otherwise it is a RECORD_TYPE.
+ QUAL_RECORD_TYPE is not supported. */
+
+record_builder::record_builder (bool is_union)
+: building_ (lang_hooks.types.make_type (is_union ? UNION_TYPE : RECORD_TYPE)),
+ last_field_ (NULL)
+{
+}
+
+
+/* Add a field with an identifier IDENT and type TYPE to the record. */
+
+void
+record_builder::add_field (tree ident, tree type, source_location loc)
+{
+ tree this_field = build_decl (loc, FIELD_DECL, ident, type);
+ DECL_CONTEXT (this_field) = building_;
+ if (last_field_)
+DECL_CHAIN (last_field_) = this_field;
+ else
+TYPE_FIELDS (building_) = this_field;
+ last_field_ = this_field;
+}
+
+void
+record_builder::add_field (const char *ident, tree type, source_location loc)
+{
+ add_field (get_identifier (ident), type, loc);
+}
+
+
+/* Add a TYPE_NAME to the record. This can be a tag name directly from IDENT,
+ or a TYPE_DECL created with the IDENT. */
+
+void
+record_builder::tag_name (tree ident)
+{
+ gcc_assert (TREE_CODE (ident) == IDENTIFIER_NODE);
+ TYPE_NAME (building_) = ident;
+}
+
+void
+record_builder::tag_name (const char *ident)
+{
+ tag_name (get_identifier (ident));
+}
+
+void
+record_builder::decl_name (tree ident, source_location loc)
+{
+ tree type_decl = build_decl (loc, TYPE_DECL, ident, building_);
+ TYPE_NAME (building_) = type_decl;
+}
+
+void
+record_builder::decl_name (const char *ident, source_location loc)
+{
+ decl_name (get_identifier (ident), loc);
+}
+
+
+/* Layout the fields of the record, aligning with ALIGN_TYPE if given.
+ Ensure that you call one of these functions after adding all fields. */
+
+void
+record_builder::layout ()
+{
+ layout_type (building_);
+}
+
+void
+record_builder::layout (tree align_type)
+{
+ TYPE_ALIGN (building_) = TYPE_ALIGN (align_type);
+ TYPE_USER_ALIGN (building_) = TYPE_USER_ALIGN (align_type);
+ layout ();
+}
+
+
+/* Return the record as a tree. You may call this function any time after
+ construction of the builder. */
+
+tree
+record_builder::as_tree ()
+{
+ return building_;
+}
+
+
#include gt-tree.h
Index: gcc/tree.h
===
--- gcc/tree.h (revision 195904)
+++ gcc/tree.h (working copy)
@@ -6529,4 +6529,28 @@ builtin_decl_implicit_p (enum built_in_f
builtin_info.implicit_p[uns_fncode]);
}
+
+/* A class for simplifying the construction of RECORD_TYPE and UNION_TYPE. */
+
+class record_builder
+{
+public:
+ record_builder (bool is_union = false);
+ void add_field (tree ident, tree type,
+ source_location loc = UNKNOWN_LOCATION);
+ void add_field (const char *ident, tree type,
+ source_location loc = UNKNOWN_LOCATION);
+ void layout ();
+ void layout (tree align_type);
+ void tag_name (tree ident);
+ void tag_name (const char *ident);
+ void decl_name (tree ident, source_location loc = UNKNOWN_LOCATION);
+ void decl_name (const char *ident, source_location loc = UNKNOWN_LOCATION);
+ tree as_tree ();
+private:
+ tree building_;
+ tree last_field_;
+}; // class record_builder
+
+
#endif /* GCC_TREE_H */
--
Lawrence Crowl
Re: [cxx-conversion] Add Record Builder Class
On 2/13/13, Diego Novillo dnovi...@google.com wrote:
On Tue, Feb 12, 2013 at 2:47 PM, Lawrence Crowl cr...@google.com wrote:
@@ -182,24 +163,9 @@ default_emutls_var_init (tree to, tree d
static tree
get_emutls_object_type (void)
{
- tree type, type_name, field;
-
- type = emutls_object_type;
- if (type)
-return type;
-
- emutls_object_type = type = lang_hooks.types.make_type (RECORD_TYPE);
- type_name = NULL;
- field = targetm.emutls.var_fields (type, type_name);
- if (!type_name)
-type_name = get_identifier (__emutls_object);
- type_name = build_decl (UNKNOWN_LOCATION,
- TYPE_DECL, type_name, type);
- TYPE_NAME (type) = type_name;
- TYPE_FIELDS (type) = field;
- layout_type (type);
-
- return type;
+ if (!emutls_object_type)
+emutls_object_type = targetm.emutls.object_type ();
+ return emutls_object_type;
Hm, this does not look like an idempotent change. Where did I
get lost?
The responsibility for creating the type has moved into the new hook.
The old hook only had responsibility for adding fields. And changing
the name if it wasn't right. The new structure has a much clearer
division of responsibility.
===
--- gcc/coverage.c (revision 195904)
+++ gcc/coverage.c (working copy)
@@ -121,8 +121,8 @@ static const char *const ctr_names[GCOV_
/* Forward declarations. */
static void read_counts_file (void);
static tree build_var (tree, tree, int);
-static void build_fn_info_type (tree, unsigned, tree);
-static void build_info_type (tree, tree);
+static tree build_fn_info_type (unsigned, tree);
+static tree build_info_type (record_builder , tree);
I don't really like unnecessary forward declarations. But I guess
it's OK, since you are replacing existing ones.
Yeah. I figure applying style changes should be a separate patch.
-static void
-build_fn_info_type (tree type, unsigned counters, tree gcov_info_type)
+static tree
+build_fn_info_type (unsigned counters, tree gcov_info_type)
So, here you are changing the signature because the caller used
to create an empty record and now it doesn't need to? We are
going to be creating it in the constructor, right?
Correct.
--
Lawrence Crowl
Re: [cxx-conversion] Add Record Builder Class
On 2/13/13, Diego Novillo dnovi...@google.com wrote: Thanks. The patch is OK for the branch. You can address Nathan's review after he's back and gets a chance to look at it. Let me know when the patch is in. I've got another merge in process. Committed. -- Lawrence Crowl
Re: [cxx-conversion] Add Record Builder Class
On 2/14/13, Richard Biener richard.guent...@gmail.com wrote:
On Tue, Feb 12, 2013 at 8:47 PM, Lawrence Crowl cr...@google.com wrote:
Add class record_builder to ease construction of records and unions. Use
it
in some appropriate places.
tree
-default_emutls_var_fields (tree type, tree *name ATTRIBUTE_UNUSED)
+default_emutls_object_type (void)
{
- tree word_type_node, field, next_field;
-
- field = build_decl (UNKNOWN_LOCATION,
- FIELD_DECL, get_identifier (__templ),
ptr_type_node);
- DECL_CONTEXT (field) = type;
- next_field = field;
-
- field = build_decl (UNKNOWN_LOCATION,
- FIELD_DECL, get_identifier (__offset),
- ptr_type_node);
- DECL_CONTEXT (field) = type;
- DECL_CHAIN (field) = next_field;
- next_field = field;
-
- word_type_node = lang_hooks.types.type_for_mode (word_mode, 1);
- field = build_decl (UNKNOWN_LOCATION,
- FIELD_DECL, get_identifier (__align),
- word_type_node);
- DECL_CONTEXT (field) = type;
- DECL_CHAIN (field) = next_field;
- next_field = field;
-
- field = build_decl (UNKNOWN_LOCATION,
- FIELD_DECL, get_identifier (__size),
word_type_node);
- DECL_CONTEXT (field) = type;
- DECL_CHAIN (field) = next_field;
-
- return field;
+ tree word_type_node = lang_hooks.types.type_for_mode (word_mode, 1);
+ record_builder rec;
+ rec.add_field (__size, word_type_node);
+ rec.add_field (__align, word_type_node);
+ rec.add_field (__offset, ptr_type_node);
+ rec.add_field (__templ, ptr_type_node);
+ rec.layout ();
That's awkward - you want to hide the fact that layout has
to happen and that it has to happen last. Just make it a
side-effect of .as_tree ().
Sometimes you want to construct recursive types, and for that you
need .as_tree to execute before layout. This feature is used in
the patch.
Note that add_field want's to return the FIELD_DECL created,
people may want to alter it.
Do you have a use case? Until we have one, I'm not convinced that
we should widen the interface.
Note that tag_name does not allow the way C++ uses this (it can
be a TYPE_DECL).
That is what the .decl_name member function does.
Overall I'm not sure this is a good abstraction unless you manage
to make the frontends use it.
The intent is for use by the middle/back ends constructing code.
As such, it should be using middle/back end types, not front-end
types.
--
Lawrence Crowl
[pph] Buffer overrun in preprocessor symbol replay
In my last PPH change, I eliminated the redundancy in the preprocessor
identifier lookaside table by removing the name of the identifier from
the head of the macro value. This later led to a buffer overrun in
libcpp/symtab.c cpp_lt_replay. The buffer was allocated based on the
value string size, which is was no longer large enough to hold the
definition string.
Split cpp_idents_used::max_length and cpp_lookaside::max_length into
max_ident_len and max_value_len. In cpp_lt_replay, allocate the
buffer based on the sum of max_ident_len and max_value_len.
--
Lawrence Crowl
src.change
Description: Binary data
Index: gcc/cp/pph.c
===
*** gcc/cp/pph.c (revision 170837)
--- gcc/cp/pph.c (working copy)
*** pth_dump_identifiers (FILE *stream, cpp_
*** 502,509
{
unsigned int idx, col = 1;
! fprintf (stream, %u identifiers up to %u chars\n,
!identifiers-num_entries, identifiers-max_length);
for (idx = 0; idx identifiers-num_entries; ++idx)
{
cpp_ident_use *ident = identifiers-entries + idx;
--- 502,510
{
unsigned int idx, col = 1;
! fprintf (stream, %u identifiers up to %u chars, vals to %u chars\n,
!identifiers-num_entries, identifiers-max_ident_len,
!identifiers-max_value_len);
for (idx = 0; idx identifiers-num_entries; ++idx)
{
cpp_ident_use *ident = identifiers-entries + idx;
*** pth_save_identifiers (cpp_idents_used *i
*** 793,814
unsigned int num_entries, id;
num_entries = identifiers-num_entries;
! pph_output_uint (stream, identifiers-max_length);
pph_output_uint (stream, num_entries);
for ( id = 0; id num_entries; ++id )
{
cpp_ident_use *entry = identifiers-entries + id;
! gcc_assert (entry-ident_len = identifiers-max_length);
pph_output_string_with_length (stream, entry-ident_str,
entry-ident_len);
! gcc_assert (entry-before_len = identifiers-max_length);
pph_output_string_with_length (stream, entry-before_str,
entry-before_len);
! gcc_assert (entry-after_len = identifiers-max_length);
pph_output_string_with_length (stream, entry-after_str,
entry-after_len);
}
--- 794,816
unsigned int num_entries, id;
num_entries = identifiers-num_entries;
! pph_output_uint (stream, identifiers-max_ident_len);
! pph_output_uint (stream, identifiers-max_value_len);
pph_output_uint (stream, num_entries);
for ( id = 0; id num_entries; ++id )
{
cpp_ident_use *entry = identifiers-entries + id;
! gcc_assert (entry-ident_len = identifiers-max_ident_len);
pph_output_string_with_length (stream, entry-ident_str,
entry-ident_len);
! gcc_assert (entry-before_len = identifiers-max_value_len);
pph_output_string_with_length (stream, entry-before_str,
entry-before_len);
! gcc_assert (entry-after_len = identifiers-max_value_len);
pph_output_string_with_length (stream, entry-after_str,
entry-after_len);
}
*** static void
*** 1025,1035
pth_load_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
{
unsigned int j;
! unsigned int max_length, num_entries;
unsigned int ident_len, before_len, after_len;
! max_length = pph_input_uint (stream);
! identifiers-max_length = max_length;
num_entries = pph_input_uint (stream);
identifiers-num_entries = num_entries;
identifiers-entries = XCNEWVEC (cpp_ident_use, num_entries);
--- 1027,1039
pth_load_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
{
unsigned int j;
! unsigned int max_ident_len, max_value_len, num_entries;
unsigned int ident_len, before_len, after_len;
! max_ident_len = pph_input_uint (stream);
! identifiers-max_ident_len = max_ident_len;
! max_value_len = pph_input_uint (stream);
! identifiers-max_value_len = max_value_len;
num_entries = pph_input_uint (stream);
identifiers-num_entries = num_entries;
identifiers-entries = XCNEWVEC (cpp_ident_use, num_entries);
Index: libcpp/symtab.c
===
*** libcpp/symtab.c (revision 170837)
--- libcpp/symtab.c (working copy)
*** cpp_lt_create (unsigned int order, unsig
*** 411,417
table-sticky_order = order;
table-active = 0;
! table-max_length = 0;
table-strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (table-strings, 0, 0,
--- 411,418
table-sticky_order = order;
table-active = 0;
! table-max_ident_len = 0;
! table-max_value_len = 0;
table-strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (table-strings, 0, 0,
*** lt_macro_value (const char** string, cpp
*** 556,563
[pph] Macro Validation (issue4379044)
In my last PPH change, I eliminated the redundancy in the preprocessor
identifier lookaside table by removing the name of the identifier from
the head of the macro value. This later led to a buffer overrun in
libcpp/symtab.c cpp_lt_replay. The buffer was allocated based on the
value string size, which is was no longer large enough to hold the
definition string.
Split cpp_idents_used.max_length and cpp_lookaside.max_length into
max_ident_len and max_value_len. In cpp_lt_replay, allocate the
buffer based on the sum of max_ident_len and max_value_len.
The simple macro validation scheme for PTH is not sufficient for
PPH. In particular, in libcpp, even identifiers that are skipped
in preprocessing are entered into the symbol table. We need to
ignore these. So, add two macro attributes, used_by_directive and
expanded_to_text. If neither of these attributes is set, then the
macro is not used and can be ignored for validation and replay.
These changes bring the macro validation to a workable state. There
may be some fine tuning later.
Make an inability to open a PPH file for reading a plain error rather
than a fatal error. Fatal errors do not seem to play well with tests.
Adjust tests to reflect changes. There is still one unexpected
failure, p1mean.cc does not compare equal in assembly. Not yet
investigated.
gcc/testsuite/ChangeLog.pph
2011-04-06 Lawrence Crowl cr...@google.com
* g++.dg/pph/p1mean.cc: Now pass validation.
* g++.dg/pph/p1stdlib.cc: Likewise.
* g++.dg/pph/d1symnotinc.cc: Failure to read is now not fatal.
* g++.dg/pph/d1chained.cc: Miscategorized as a failure, ...
* g++.dg/pph/c1chained.cc: so it has been renamed.
gcc/cp/ChangeLog.pph
2011-04-06 Lawrence Crowl cr...@google.com
* pph.c (pth_dump_identifiers): Split cpp_idents_used::max_length
into max_ident_length and max_value_length. Print used_by_directive
and expanded_to_text attributes of macros.
(pth_save_identifiers): Split cpp_idents_used::max_length into
max_ident_length and max_value_length. Filter out macro that are
neither used_by_directive nor expanded_to_text, which requires
precounting the number of entries remaining. Save used_by_directive
and expanded_to_text attributes of the macros.
(pth_load_identifiers): Split cpp_idents_used::max_length into
max_ident_length and max_value_length. Restore used_by_directive and
expanded_to_text attributes of the macros.
(pph_read_file): Make failure to read a pph file a non-fatal error.
libcpp/ChangeLog.pph
2011-04-06 Lawrence Crowl cr...@google.com
* include/cpplib.h (struct cpp_hashnode): Add used_by_directive and
expanded_to_text attributes for macros. Take their bits from
directive_index, which does not need them.
* include/symtab.h (struct cpp_idents_used): Split max_length into
max_ident_len and max_value_len. Reorder fields to avoid gaps.
(struct cpp_ident_use): Add used_by_directive and expanded_to_text
attributes for macros.
* internal.h (struct cpp_lookaside): Split max_length into
max_ident_len and max_value_len.
* macro.c (enter_macro_context): Mark expanded_to_text macro attribute.
* directives.c (DIRECTIVE_TABLE): Add sizing comment.
(do_define): Mark used_by_directive macro attribute.
(do_undef): Likewise.
(do_ifdef): Likewise.
* expr.c (parse_defined): Mark used_by_directive macro attribute.
* symtab.c (cpp_lt_create): Split cpp_lookaside::max_length into
max_ident_len and max_value_len.
* (lt_macro_value): Likewise.
* (lt_lookup): Likewise.
* (cpp_lt_capture): Likewise. Also save used_by_directive and
expanded_to_text attributes of macros.
* (cpp_lt_replay): Split cpp_idents_used::max_lenth into
max_ident_len and max_value_len. Allocate a buffer with the sum.
Index: gcc/testsuite/g++.dg/pph/p1mean.cc
===
*** gcc/testsuite/g++.dg/pph/p1mean.cc (revision 172001)
--- gcc/testsuite/g++.dg/pph/p1mean.cc (working copy)
***
*** 1,9
! #include stdlib.h // {dg-error fails macro validation { xfail *-*-* } }
! #include stdio.h // {dg-error fails macro validation { xfail *-*-* } }
! #include math.h // {dg-error fails macro validation { xfail *-*-* } }
! #include string.h // {dg-error fails macro validation { xfail *-*-* } }
! // { dg-excess-errors In file included from { xfail *-*-* } }
! // { dg-excess-errors assembly comparison { xfail *-*-* } }
static unsigned long long MAX_ITEMS = 1;
--- 1,7
! #include stdlib.h
! #include stdio.h
! #include math.h
! #include string.h
static unsigned long long MAX_ITEMS = 1;
Index: gcc/testsuite/g++.dg/pph/d1symnotinc.cc
[pph] Test PPH include at global scope. (issue4399041)
Add a test to ensure that PPH files are #included at global scope.
Initially, this test is XFAIL, as it's a low priority error.
Index: gcc/testsuite/ChangeLog.pph
2011-04-12 Lawrence Crowl cr...@google.com
* g++.dg/pph/y2smother.cc: New.
Index: gcc/testsuite/g++.dg/pph/y2smother.cc
===
*** gcc/testsuite/g++.dg/pph/y2smother.cc (revision 0)
--- gcc/testsuite/g++.dg/pph/y2smother.cc (revision 0)
***
*** 0
--- 1,4
+ namespace smother {
+ #include x1struct1.h
+ // { dg-error pph file not included at global scope { xfail *-*-* } }
+ }
--
This patch is available for review at http://codereview.appspot.com/4399041
Re: [pph] Pickle some more language-dependent fields (issue4389050)
(stream, bl-usings, ref_p);
- pph_output_chain (stream, bl-using_directives, ref_p);
+ pph_output_chain_filtered (stream, bl-usings, ref_p, NO_BUILTINS);
+ pph_output_chain_filtered (stream, bl-using_directives, ref_p,
NO_BUILTINS);
pph_output_uint (stream, VEC_length (cp_class_binding,
bl-class_shadowed));
for (i = 0; VEC_iterate (cp_class_binding, bl-class_shadowed, i, cs);
i++)
@@ -594,6 +614,8 @@ pph_stream_write_tree (struct output_block *ob, tree
expr, bool ref_p)
if (TREE_CODE (expr) == FUNCTION_DECL)
pph_output_tree_aux (stream, DECL_SAVED_TREE (expr), ref_p);
}
+ else if (TREE_CODE (expr) == TYPE_DECL)
+ pph_output_tree (stream, DECL_ORIGINAL_TYPE (expr), ref_p);
}
else if (TREE_CODE (expr) == STATEMENT_LIST)
{
@@ -611,3 +633,52 @@ pph_stream_write_tree (struct output_block *ob, tree
expr, bool ref_p)
pph_output_tree_aux (stream, tsi_stmt (i), ref_p);
}
}
+
+
+/* Output a chain of nodes to STREAM starting with FIRST. Skip any
+ nodes that do not match FILTER. REF_P is true if nodes in the chain
+ should be emitted as references. */
+
+void
+pph_output_chain_filtered (pph_stream *stream, tree first, bool ref_p,
+enum chain_filter filter)
+{
+ unsigned count;
+ tree t;
+
+ /* Special case. If the caller wants no filtering, it is much
+ faster to just call pph_output_chain directly. */
+ if (filter == NONE)
+{
+ pph_output_chain (stream, first, ref_p);
+ return;
+}
+
+ /* Count all the nodes that match the filter. */
+ for (t = first, count = 0; t; t = TREE_CHAIN (t))
+{
+ if (filter == NO_BUILTINS DECL_P (t) DECL_IS_BUILTIN (t))
+ continue;
+ count++;
+}
+ pph_output_uint (stream, count);
+
+ /* Output all the nodes that match the filter. */
+ for (t = first; t; t = TREE_CHAIN (t))
+{
+ tree saved_chain;
+
+ /* Apply filters to T. */
+ if (filter == NO_BUILTINS DECL_P (t) DECL_IS_BUILTIN (t))
+ continue;
+
+ /* Clear TREE_CHAIN to avoid blindly recursing into the rest
+ of the list. */
+ saved_chain = TREE_CHAIN (t);
+ TREE_CHAIN (t) = NULL_TREE;
+
+ pph_output_tree (stream, t, ref_p);
+
+ TREE_CHAIN (t) = saved_chain;
+}
+}
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index e65a792..91e01a4 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -146,8 +146,14 @@ pph_stream_trace (pph_stream *stream, const void *data,
unsigned int nbytes,
case PPH_TRACE_TREE:
{
const_tree t = (const_tree) data;
- print_generic_expr (pph_logfile, CONST_CAST (union tree_node *, t),
- TDF_SLIM);
+ if (t)
+ {
+ print_generic_expr (pph_logfile, CONST_CAST (union tree_node *, t),
+ 0);
+ fprintf (pph_logfile, , code=%s, tree_code_name[TREE_CODE (t)]);
+ }
+ else
+ fprintf (pph_logfile, NULL_TREE);
}
break;
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 8731eb8..329e507 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -87,6 +87,9 @@ typedef struct pph_stream {
unsigned int write_p : 1;
} pph_stream;
+/* Filter values for pph_output_chain_filtered. */
+enum chain_filter { NONE, NO_BUILTINS };
+
/* In pph-streamer.c. */
pph_stream *pph_stream_open (const char *, const char *);
void pph_stream_close (pph_stream *);
@@ -104,6 +107,7 @@ void pph_stream_init_write (pph_stream *);
void pph_stream_write_tree (struct output_block *, tree, bool ref_p);
void pph_stream_pack_value_fields (struct bitpack_d *, tree);
void pph_stream_output_tree_header (struct output_block *, tree);
+void pph_output_chain_filtered (pph_stream *, tree, bool, enum
chain_filter);
/* In pph-streamer-in.c. */
void pph_stream_init_read (pph_stream *);
--
This patch is available for review at http://codereview.appspot.com/4389050
--
Lawrence Crowl
Re: [pph] Reconstruct a header file from a pph image (issue4392047)
LGTM
On 4/12/11, Diego Novillo dnovi...@google.com wrote:
This patch, together with the others I sent today, allows us to
reconstruct all the symbols and types stored inside a pph image. This
is not all we need, but it allows me to get basic declarations and
types reconstructed from pph images.
This causes ~57 failures in pph.exp. The more problematic one is an
infinite recursion that causes memory bloat. I'm not committing this
patch yet until I figure this one out.
Other failures are due to missing bits in the pickling. I'll be
fixing those in subsequent patches.
2011-04-12 Diego Novillo dnovi...@google.com
* pph.c (pph_add_names_to_namespace): New.
(pph_read_file_contents): Call it.
Call cpp_lt_replay.
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
index 74d1d50..6584e72 100644
--- a/gcc/cp/pph.c
+++ b/gcc/cp/pph.c
@@ -1937,6 +1937,28 @@ report_validation_error (const char *filename,
}
+
+/* Add all the new names declared in NEW_NS to NS. */
+
+static void
+pph_add_names_to_namespace (tree ns, tree new_ns)
+{
+ struct cp_binding_level *new_level, *level;
+ tree t, chain;
+
+ level = NAMESPACE_LEVEL (ns);
+ new_level = NAMESPACE_LEVEL (new_ns);
+
+ for (t = new_level-names; t; t = chain)
+{
+ /* Pushing a decl into a scope clobbers its DECL_CHAIN.
+ Preserve it. */
+ chain = DECL_CHAIN (t);
+ pushdecl_with_scope (t, level, /*is_friend=*/false);
+}
+}
+
+
/* Read contents of PPH file in STREAM. */
static void
@@ -1946,25 +1968,27 @@ pph_read_file_contents (pph_stream *stream)
cpp_ident_use *bad_use;
const char *cur_def;
cpp_idents_used idents_used;
+ tree file_ns;
pth_load_identifiers (idents_used, stream);
- /*FIXME pph: This validation is weak. */
+ /* FIXME pph: This validation is weak. */
verified = cpp_lt_verify_1 (parse_in, idents_used, bad_use, cur_def,
true);
if (!verified)
report_validation_error (stream-name, bad_use-ident_str, cur_def,
bad_use-before_str, bad_use-after_str);
- /* FIXME pph: We cannot replay the macro definitions
- as long as we are still reading the actual file.
+ /* Re-instantiate all the pre-processor symbols defined by STREAM. */
cpp_lt_replay (parse_in, idents_used);
- */
- pph_input_tree (stream);
+ /* Read global_namespace from STREAM and add all the names defined
+ there to the current global_namespace. */
+ file_ns = pph_input_tree (stream);
+ pph_add_names_to_namespace (global_namespace, file_ns);
}
-/* Read PPH file. */
+/* Read PPH file FILENAME. */
static void
pph_read_file (const char *filename)
--
This patch is available for review at http://codereview.appspot.com/4392047
--
Lawrence Crowl
Re: [pph] Stream merging information (issue 5090041)
On 9/21/11, dnovi...@google.com dnovi...@google.com wrote: http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c#newcode2146 gcc/cp/pph-streamer-in.c:2146: pph_read_namespace_chain (pph_stream *stream, tree enclosing_namespace) 2142 /* Read a chain of tree nodes from input block IB. DATA_IN contains 2143tables and descriptors for the file being read. */ 2144 2145 tree 2146 pph_read_namespace_chain (pph_stream *stream, tree enclosing_namespace) ENCLOSING_NAMESPACE needs documenting. Copy/paste/edit failure. Would it be better to have the original pph_read_chain() get this argument? Not crazy about this duplication of code. There is no origional pph_read_chain. The pph_in_chain uses streamer_read_chain. I didn't think altering that API was the right thing to do for a pph-specific feature. Same comment applies to the other two routines that the patch duplicates. In the end, I decided that the duplication was likely to be worth it to avoid all the pointer passing and checking. Most trees streamed will not be direct children of namespaces. -- Lawrence Crowl
[pph] More merging. (issue5222045)
Merge symbols. This patch doesn't actually fix anything, but it does
reorganize to address known issues. In particular, the code shifts
from merging into a namespace to merging into a particular chain.
This prevents a latent bug in merging into inappropriate chains. It
also sets us up to merge structs, which we may need due to users
binding directly to members.
Index: gcc/testsuite/ChangeLog.pph
2011-10-06 Lawrence Crowl cr...@google.com
* g++.dg/pph/c4inline.cc: Clarify failure.
* g++.dg/pph/x4keyno.cc: Clarify failure.
* g++.dg/pph/x4keyex.cc: Clarify failure.
* g++.dg/pph/x4keyed.cc: Clarify failure.
* g++.dg/pph/x0tmplclass23.h: Fix header guard variables.
Index: gcc/cp/ChangeLog.pph
2011-10-06 Lawrence Crowl cr...@google.com
* pt.c (pph_in_pending_templates_list): Send pph logging to
the pph log file.
* pph.c (pph_tree_code_text): Correct specification of enum bound.
* pph-streamer-in.c (pph_in_namespace_tree_vec): Remove.
(pph_in_binding_level): Change to mutator. Mutate early so that
the result is usable while reading sub-trees.
(pph_in_tcc_declaration): Comment on redundancy.
(pph_match_to_overload): New.
(pph_match_to_function): New.
(pph_match_to_link): New.
(pph_search_in_chain): New.
(pph_prepend_to_chain): New.
(pph_merge_into_chain): New.
(pph_read_namespace_tree): Rename to pph_read_any_tree. Mutate
a specific chain rather than mutate a namespace.
(pph_read_tree): Move within source.
(pph_read_mergeable_tree): New.
(pph_read_mergeable_chain): Chaining is now done by
pph_in_mergeable_tree.
* pph-streamer-out.c (pph_out_namespace_tree_vec): Rename to
pph_out_mergeable_tree_vec. Remove unneeded namespace parameter.
(pph_out_namespace_tree_vec_filtered): Remove unneeded.
(pph_write_namespace_chain): Replace with pph_write_mergeable_chain.
(pph_write_mergeable_links): New.
(pph_out_namespace_chain_filtered): Rename to
pph_out_mergeable_chain_filtered. Remove unneeded namespace parameter.
(pph_out_binding_level_1): Adjust code to track the above changes.
-static_decls are already in declaration order, factor them out of
mergable tests.
(pph_out_tcc_declaration): Comment on redundancy.
(pph_out_tcc_type): Comment on redundancy.
(pph_write_namespace_tree): Rename to pph_write_any_tree.
Change namespace parameter to boolean. Emit location info for merging.
(pph_write_mergeable_tree): New.
* pph-streamer.h: Modify function declarations to reflect the above.
(pph_out_namespace_tree): Rename to pph_out_mergeable_tree. Remove
unneeded namespace argument.
(pph_out_namespace_tree_1): Fold into pph_out_mergeable_tree.
(pph_out_namespace_chain): Rname to pph_out_mergeable_chain. Remove
unneeded namespace argument.
(pph_in_namespace_tree): Rname to pph_in_mergeable_tree. Change
argument to chain instead of namespace.
(pph_in_namespace_chain): Rname to pph_in_mergeable_chain. Change
argument to chain instead of namespace.
Index: gcc/testsuite/g++.dg/pph/x4keyno.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyno.cc (revision 179580)
+++ gcc/testsuite/g++.dg/pph/x4keyno.cc (working copy)
@@ -1,4 +1,4 @@
-// { dg-xfail-if BOGUS { *-*-* } { -fpph-map=pph.map } }
+// { dg-xfail-if BOGUS MERGE AUXVAR { *-*-* } { -fpph-map=pph.map } }
// { dg-bogus x4keyno.cc:11:1: error: redefinition of 'const char _ZTS5keyno
{ xfail *-*-* } 0 }
#include x0keyno1.h
Index: gcc/testsuite/g++.dg/pph/c4inline.cc
===
--- gcc/testsuite/g++.dg/pph/c4inline.cc(revision 179580)
+++ gcc/testsuite/g++.dg/pph/c4inline.cc(working copy)
@@ -1,6 +1,8 @@
// pph asm xdiff 36250
-//Emitting a second copy of the inline function f.
-//With comdat, the linker may paper over the differences.
+// xfail BOGUS DUPFUN
+//
+// Emitting a second copy of the inline function f.
+// With comdat, the linker may paper over the differences.
#include c0inline1.h
#include c0inline2.h
Index: gcc/testsuite/g++.dg/pph/x4keyex.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyex.cc (revision 179580)
+++ gcc/testsuite/g++.dg/pph/x4keyex.cc (working copy)
@@ -1,4 +1,5 @@
// pph asm xdiff 32642
+// xfail BOGUS LABELS
//
// This test case fails to compare because LFB/LFE labels are different.
//
Index: gcc/testsuite/g++.dg/pph/x4keyed.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyed.cc (revision 179580)
+++ gcc/testsuite/g++.dg/pph/x4keyed.cc (working copy)
@@ -1,4 +1,4 @@
-// { dg-xfail-if BOGUS
[pph] De-inline streamer functions. (issue5245043)
Move several static inline functions from pph-streamer.h to
pph-streamer-{out,in}.c. This is part 1 of several patches,
and minimizes diffs.
Index: gcc/cp/ChangeLog.pph
2011-10-07 Lawrence Crowl cr...@google.com
* pph-streamer.h (pph_out_tree_array): Remove unused.
(pph_in_tree_array): Remove unused.
(pph_in_tree_VEC): Remove unused.
(pph_out_uint): Moved to pph-streamer-out.c as extern.
(pph_out_record_marker): Likewise.
(pph_out_chain): Moved to pph-streamer-out.c as static.
(pph_out_mergeable_chain): Likewise.
(pph_out_bitpack): Likewise.
(pph_in_uint): Moved to pph-streamer-in.c as extern.
(pph_in_location): Likewise.
(pph_in_tree): Likewise.
(pph_in_record_marker): Likewise.
(pph_in_uhwi): Moved to pph-streamer-in.c as static.
(pph_in_hwi): Likewise.
(pph_in_uchar): Likewise.
(pph_in_bytes): Likewise.
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179636)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -146,6 +146,89 @@ pph_init_read (pph_stream *stream)
}
+/* Read an unsigned char VALUE to STREAM. */
+static unsigned char
+pph_in_uchar (pph_stream *stream)
+{
+ unsigned char n = streamer_read_uchar (stream-encoder.r.ib);
+ if (flag_pph_tracer = 4)
+pph_trace_uint (stream, n);
+ return n;
+}
+
+/* Read a HOST_WIDE_INT from STREAM. */
+static inline HOST_WIDE_INT
+pph_in_hwi (pph_stream *stream)
+{
+ return streamer_read_hwi (stream-encoder.r.ib);
+}
+
+
+/* Read an unsigned HOST_WIDE_INT from STREAM. */
+static inline unsigned HOST_WIDE_INT
+pph_in_uhwi (pph_stream *stream)
+{
+ return streamer_read_uhwi (stream-encoder.r.ib);
+}
+
+
+/* Read an unsigned integer from STREAM. */
+unsigned int
+pph_in_uint (pph_stream *stream)
+{
+ HOST_WIDE_INT unsigned n = streamer_read_uhwi (stream-encoder.r.ib);
+ gcc_assert (n == (unsigned) n);
+ if (flag_pph_tracer = 4)
+pph_trace_uint (stream, n);
+ return (unsigned) n;
+}
+
+
+/* Read N bytes from STREAM into P. The caller is responsible for
+ allocating a sufficiently large buffer. */
+static void
+pph_in_bytes (pph_stream *stream, void *p, size_t n)
+{
+ lto_input_data_block (stream-encoder.r.ib, p, n);
+ if (flag_pph_tracer = 4)
+pph_trace_bytes (stream, p, n);
+}
+
+
+/* Read and return a string from STREAM. */
+
+static const char *
+pph_in_string (pph_stream *stream)
+{
+ const char *s = streamer_read_string (stream-encoder.r.data_in,
+stream-encoder.r.ib);
+ if (flag_pph_tracer = 4)
+pph_trace_string (stream, s);
+ return s;
+}
+
+
+/* Read and return a record marker from STREAM. On return, *TAG_P will
+ contain the tag for the data type stored in this record. */
+enum pph_record_marker
+pph_in_record_marker (pph_stream *stream, enum pph_tag *tag_p)
+{
+ enum pph_record_marker m = (enum pph_record_marker) pph_in_uchar (stream);
+ gcc_assert (m == PPH_RECORD_START
+ || m == PPH_RECORD_START_NO_CACHE
+ || m == PPH_RECORD_START_MUTATED
+ || m == PPH_RECORD_END
+ || m == PPH_RECORD_IREF
+ || m == PPH_RECORD_XREF
+ || m == PPH_RECORD_PREF);
+
+ *tag_p = (enum pph_tag) pph_in_uint (stream);
+ gcc_assert ((unsigned) *tag_p (unsigned) PPH_NUM_TAGS);
+
+ return m;
+}
+
+
/* Read and return a record header from STREAM. EXPECTED_TAG indicates
the data type that should be stored in this record. When a
PPH_RECORD_START marker is read, the next word read is an index
@@ -237,6 +320,22 @@ pph_read_location (struct lto_input_bloc
}
+/* Read and return a location_t from STREAM.
+ FIXME pph: If pph_trace didn't depend on STREAM, we could avoid having to
+ call this function, only for it to call lto_input_location, which calls the
+ streamer hook back to pph_read_location. */
+
+location_t
+pph_in_location (pph_stream *stream)
+{
+ location_t loc = pph_read_location (stream-encoder.r.ib,
+ stream-encoder.r.data_in);
+ if (flag_pph_tracer = 4)
+pph_trace_location (stream, loc);
+ return loc;
+}
+
+
/* Load the tree value associated with TOKEN from STREAM. */
static void
@@ -2280,6 +2379,16 @@ pph_read_tree (struct lto_input_block *i
}
+/* Load an AST from STREAM. Return the corresponding tree. */
+tree
+pph_in_tree (pph_stream *stream)
+{
+ tree t = pph_read_any_tree (stream, NULL);
+ if (flag_pph_tracer = 4)
+pph_trace_tree (stream, t);
+ return t;
+}
+
/* Read a mergeable tree from STREAM into CHAIN. */
tree
Index: gcc/cp/pph-streamer.h
===
--- gcc/cp/pph-streamer.h (revision 179636)
+++ gcc/cp/pph-streamer.h (working copy)
@@ -439,21 +439,6 @@ pph_cache_find (pph_stream *stream, enum
return e-data
De-inline streamer functions part 2 (issue5250042)
Move several static inline functions from pph-streamer.h to
pph-streamer-{out,in}.c. This is part 2 of several patches,
and minimizes diffs.
Index: gcc/cp/ChangeLog.pph
2011-10-07 Lawrence Crowl cr...@google.com
* pph-streamer.h (pph_in_chain): Moved to pph-streamer-in.c as extern.
(pph_read_mergeable_tree): Make static in pph-streamer-in.c.
(pph_read_mergeable_chain): Likewise
(pph_in_mergeable_tree): Moved to pph-streamer-in.c as static.
(pph_in_mergeable_chain): Likewise.
(pph_in_bitpack): Likewise.
(pph_in_chain): Likewise.
(pph_out_location): Moved to pph-streamer-out.c extern.
(pph_out_tree): Likewise.
(pph_out_tree_1): Moved to pph-streamer-out.c as static.
(pph_out_mergeable_tree): Likewise.
(pph_tree_code_to_tag): Comment unused.
(pph_tag_to_tree_code): Likewise.
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179673)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -208,6 +208,17 @@ pph_in_string (pph_stream *stream)
}
+/* Read a bitpack from STREAM. */
+static struct bitpack_d
+pph_in_bitpack (pph_stream *stream)
+{
+ struct bitpack_d bp = streamer_read_bitpack (stream-encoder.r.ib);
+ if (flag_pph_tracer = 4)
+pph_trace_bitpack (stream, bp);
+ return bp;
+}
+
+
/* Read and return a record marker from STREAM. On return, *TAG_P will
contain the tag for the data type stored in this record. */
enum pph_record_marker
@@ -608,6 +619,29 @@ pph_in_label_binding (pph_stream *stream
}
+/* Read a chain of ASTs from STREAM. */
+static tree
+pph_in_chain (pph_stream *stream)
+{
+ tree t = streamer_read_chain (stream-encoder.r.ib,
+stream-encoder.r.data_in);
+ if (flag_pph_tracer = 2)
+pph_trace_chain (stream, t);
+ return t;
+}
+
+
+static void pph_read_mergeable_chain (pph_stream *stream, tree* chain);
+
+
+/* Read and merge a chain of ASTs from STREAM into an existing CHAIN. */
+static inline void
+pph_in_mergeable_chain (pph_stream *stream, tree* chain)
+{
+ pph_read_mergeable_chain (stream, chain);
+}
+
+
/* Read and return an instance of cp_binding_level from STREAM.
TO_REGISTER is used when the caller wants to read a binding level,
but register a different binding level in the streaming cache.
@@ -2389,15 +2423,27 @@ pph_in_tree (pph_stream *stream)
return t;
}
+
/* Read a mergeable tree from STREAM into CHAIN. */
-tree
+static tree
pph_read_mergeable_tree (pph_stream *stream, tree *chain)
{
return pph_read_any_tree (stream, chain);
}
+/* Load an AST in an ENCLOSING_NAMESPACE from STREAM.
+ Return the corresponding tree. */
+static void
+pph_in_mergeable_tree (pph_stream *stream, tree *chain)
+{
+ tree t = pph_read_mergeable_tree (stream, chain);
+ if (flag_pph_tracer = 3)
+pph_trace_tree (stream, t);
+}
+
+
/* Read a chain of tree nodes from STREAM. */
void
Index: gcc/cp/pph-streamer-out.c
===
--- gcc/cp/pph-streamer-out.c (revision 179673)
+++ gcc/cp/pph-streamer-out.c (working copy)
@@ -204,6 +204,17 @@ pph_write_location (struct output_block
}
+/* Write location LOC of length to STREAM. */
+
+void
+pph_out_location (pph_stream *stream, location_t loc)
+{
+ if (flag_pph_tracer = 4)
+pph_trace_location (stream, loc);
+ pph_write_location (stream-encoder.w.ob, loc);
+}
+
+
/* Write a chain of ASTs to STREAM starting with FIRST. */
static void
pph_out_chain (pph_stream *stream, tree first)
@@ -609,6 +620,37 @@ pph_out_token_cache (pph_stream *f, cp_t
pph_out_token (f, tok);
}
+
+/* Output AST T to STREAM. If -fpph-tracer is set to TLEVEL or
+ higher, T is sent to pph_trace_tree. */
+static void
+pph_out_tree_1 (pph_stream *stream, tree t, int tlevel)
+{
+ if (flag_pph_tracer = tlevel)
+pph_trace_tree (stream, t);
+ pph_write_tree (stream-encoder.w.ob, t, false);
+}
+
+/* Output AST T to STREAM. Trigger tracing at -fpph-tracer=2. */
+void
+pph_out_tree (pph_stream *stream, tree t)
+{
+ pph_out_tree_1 (stream, t, 2);
+}
+
+static void pph_write_mergeable_tree (pph_stream *stream, tree expr);
+
+/* Output AST T from ENCLOSING_NAMESPACE to STREAM.
+ Trigger tracing at -fpph-tracer=2. */
+static void
+pph_out_mergeable_tree (pph_stream *stream, tree t)
+{
+ if (flag_pph_tracer = 2)
+pph_trace_tree (stream, t);
+ pph_write_mergeable_tree (stream, t);
+}
+
+
/* Write all the fields in lang_decl_base instance LDB to OB. */
static void
Index: gcc/cp/pph-streamer.h
===
--- gcc/cp/pph-streamer.h (revision 179673)
+++ gcc/cp/pph-streamer.h (working copy)
@@ -336,7 +336,9 @@ unsigned pph_get_signature (tree, size_t
void pph_flush_buffers (pph_stream *);
void pph_init_write (pph_stream *);
void
De-inline streamer functions part 3 (issue5247042)
Remove #if 0 scaffolding. This is part 3 of several patches,
and minimizes diffs.
Index: gcc/cp/ChangeLog.pph
2011-10-07 Lawrence Crowl cr...@google.com
* pph-streamer.h: Remove #if 0 scaffolding.
Index: gcc/cp/pph-streamer.h
===
--- gcc/cp/pph-streamer.h (revision 179698)
+++ gcc/cp/pph-streamer.h (working copy)
@@ -336,9 +336,6 @@ unsigned pph_get_signature (tree, size_t
void pph_flush_buffers (pph_stream *);
void pph_init_write (pph_stream *);
void pph_write_tree (struct output_block *, tree, bool);
-#if 0
-void pph_write_mergeable_tree (pph_stream *, tree);
-#endif
void pph_write_mergeable_chain (pph_stream *, tree);
void pph_add_decl_to_symtab (tree, enum pph_symtab_action, bool, bool);
void pph_add_include (pph_stream *);
@@ -354,10 +351,6 @@ struct binding_table_s *pph_in_binding_t
/* In pph-streamer-in.c. */
void pph_init_read (pph_stream *);
tree pph_read_tree (struct lto_input_block *, struct data_in *);
-#if 0
-tree pph_read_mergeable_tree (pph_stream *, tree *);
-void pph_read_mergeable_chain (pph_stream *, tree *);
-#endif
location_t pph_read_location (struct lto_input_block *, struct data_in *);
void pph_read_file (const char *);
void pph_reader_finish (void);
@@ -444,314 +437,21 @@ pph_cache_find (pph_stream *stream, enum
}
-#if 0
-/* Output AST T to STREAM. If -fpph-tracer is set to TLEVEL or
- higher, T is sent to pph_trace_tree. */
-static inline void
-pph_out_tree_1 (pph_stream *stream, tree t, int tlevel)
-{
- if (flag_pph_tracer = tlevel)
-pph_trace_tree (stream, t);
- pph_write_tree (stream-encoder.w.ob, t, false);
-}
-
-/* Output AST T to STREAM. Trigger tracing at -fpph-tracer=2. */
-static inline void
-pph_out_tree (pph_stream *stream, tree t)
-{
- pph_out_tree_1 (stream, t, 2);
-}
-
-/* Output AST T from ENCLOSING_NAMESPACE to STREAM.
- Trigger tracing at -fpph-tracer=2. */
-static inline void
-pph_out_mergeable_tree (pph_stream *stream, tree t)
-{
- if (flag_pph_tracer = 2)
-pph_trace_tree (stream, t);
- pph_write_mergeable_tree (stream, t);
-}
-#else
extern void pph_out_tree (pph_stream *stream, tree t);
-#endif
-
-#if 0
-/* Write an unsigned int VALUE to STREAM. */
-static inline void
-pph_out_uint (pph_stream *stream, unsigned int value)
-{
- if (flag_pph_tracer = 4)
-pph_trace_uint (stream, value);
- streamer_write_uhwi (stream-encoder.w.ob, value);
-}
-
-/* Write an unsigned HOST_WIDE_INT VALUE to STREAM. */
-static inline void
-pph_out_uhwi (pph_stream *stream, unsigned HOST_WIDE_INT value)
-{
- streamer_write_uhwi (stream-encoder.w.ob, value);
-}
-
-/* Write a HOST_WIDE_INT VALUE to stream. */
-static inline void
-pph_out_hwi (pph_stream *stream, HOST_WIDE_INT value)
-{
- streamer_write_hwi (stream-encoder.w.ob, value);
-}
-
-/* Write an unsigned char VALUE to STREAM. */
-static inline void
-pph_out_uchar (pph_stream *stream, unsigned char value)
-{
- if (flag_pph_tracer = 4)
-pph_trace_uint (stream, value);
- streamer_write_char_stream (stream-encoder.w.ob-main_stream, value);
-}
-
-/* Write N bytes from P to STREAM. */
-static inline void
-pph_out_bytes (pph_stream *stream, const void *p, size_t n)
-{
- if (flag_pph_tracer = 4)
-pph_trace_bytes (stream, p, n);
- lto_output_data_stream (stream-encoder.w.ob-main_stream, p, n);
-}
-/* Write string STR to STREAM. */
-static inline void
-pph_out_string (pph_stream *stream, const char *str)
-{
- if (flag_pph_tracer = 4)
-pph_trace_string (stream, str);
- streamer_write_string (stream-encoder.w.ob,
-stream-encoder.w.ob-main_stream, str, false);
-}
-
-/* Write string STR of length LEN to STREAM. */
-static inline void
-pph_out_string_with_length (pph_stream *stream, const char *str,
- unsigned int len)
-{
- if (flag_pph_tracer = 4)
-pph_trace_string_with_length (stream, str, len);
- streamer_write_string_with_length (stream-encoder.w.ob,
-stream-encoder.w.ob-main_stream,
-str, len + 1, false);
-}
-#else
extern void pph_out_uint (pph_stream *stream, unsigned int value);
-#endif
-#if 0
-/* Write location LOC of length to STREAM. */
-static inline void
-pph_out_location (pph_stream *stream, location_t loc)
-{
- if (flag_pph_tracer = 4)
-pph_trace_location (stream, loc);
- pph_write_location (stream-encoder.w.ob, loc);
-}
-#else
extern void pph_out_location (pph_stream *stream, location_t loc);
-#endif
-
-#if 0
-/* Write a chain of ASTs to STREAM starting with FIRST. */
-static inline void
-pph_out_chain (pph_stream *stream, tree first)
-{
- if (flag_pph_tracer = 2)
-pph_trace_chain (stream, first);
- streamer_write_chain (stream-encoder.w.ob, first, false);
-}
-
-/* Write a chain of ASTs to STREAM starting with FIRST. */
-static inline void
-pph_out_mergeable_chain (pph_stream *stream, tree first
[pph] Make pph.h _the_ interface header. (issue5247044)
This patch makes pph.h the single interface header for PPH.
The pph-streamer.h header is now an internal header for use
within the pph*.c files. Some declarations have moved from
one header to the other to make that possible.
Index: gcc/cp/ChangeLog.pph
2011-10-07 Lawrence Crowl cr...@google.com
* pph-streamer.h (typedef pph_stream): Move to pph.h.
(enum pph_symtab_action): Likewise.
(enum pph_record_marker): Likewise.
(enum pph_tag): Likewise.
(extern pph_out_uint): Liekwise.
(extern pph_out_location): Likewise.
(extern pph_out_tree): Likewise.
(extern pph_out_record_marker): Likewise.
(extern pph_add_decl_to_symtab): Likewise.
(extern pph_in_uint): Likewise.
(extern pph_in_location): Likewise.
(extern pph_in_tree): Likewise.
(extern pph_record_marker): Likewise.
(extern struct binding_table_s): Likewise.
(extern pph_out_binding_table): Likewise.
(extern pph_in_binding_table): Likewise.
(extern pph_out_pending_templates_list): Likewise.
(extern pph_out_spec_entry_tables): Likewise.
(extern pph_in_pending_templates_list): Likewise.
(extern pph_in_spec_entry_tables): Likewise.
(pph_writer_enabled_p): Likewise.
(pph_enabled_p): Likewise.
* pph.h (cp_token_hunk): Remove unused.
(cp_token_hunk_ptr): Likewise.
(DEF_VEC_P (cp_token_hunk_ptr)): Likewise.
(DEF_VEC_ALLOC_P (cp_token_hunk_ptr,gc)): Likewise.
(#error diagnostic): Likewise.
(extern pph_tree_code_text): Move to pph-streamer.h.
(extern pph_dump_min_decl): Likewise.
(extern pph_dump_namespace): Likewise.
* pph.h: Remove includes for hashtab.h, line-map.h, parser.h, and
timevar.h.
* pph-streamer.h: Add include pph.h.
* pph-streamer-in.c: Add include parser.h.
* pph-streamer-out.c: Likewise.
* decl.c: Include pph.h instead of pph-streamer.h.
* semantics.c: Likewise.
* parser.c: Remove include of pph-streamer.h.
* name-lookup.c: Likewise.
* Make-lang.in: Adjust dependences to reflect the above.
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 179580)
+++ gcc/cp/decl.c (working copy)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
#include coretypes.h
#include tm.h
#include tree.h
+#include pph.h
#include flags.h
#include cp-tree.h
#include tree-iterator.h
@@ -54,7 +55,6 @@ along with GCC; see the file COPYING3.
#include pointer-set.h
#include splay-tree.h
#include plugin.h
-#include pph-streamer.h
/* Possible cases of bad specifiers type used by bad_specifiers. */
enum bad_spec_place {
Index: gcc/cp/Make-lang.in
===
--- gcc/cp/Make-lang.in (revision 179580)
+++ gcc/cp/Make-lang.in (working copy)
@@ -265,10 +265,9 @@ CXX_TREE_H = $(TREE_H) cp/name-lookup.h
$(srcdir)/../include/hashtab.h
CXX_PARSER_H = tree.h c-family/c-pragma.h cp/parser.h
CXX_PRETTY_PRINT_H = cp/cxx-pretty-print.h $(C_PRETTY_PRINT_H)
-CXX_PPH_H = $(srcdir)/../libcpp/include/line-map.h $(HASHTAB_H) \
- $(CXX_PARSER_H) $(TIMEVAR_H) $(CXX_TREE_H) cp/pph.h
-CXX_PPH_STREAMER_H = $(LTO_STREAMER_H) cp/pph-streamer.h $(TREE_H) \
- $(DATA_STREAMER_H) $(TREE_STREAMER_H)
+CXX_PPH_H = $(CXX_TREE_H) cp/pph.h
+CXX_PPH_STREAMER_H = $(LTO_STREAMER_H) $(DATA_STREAMER_H) $(TREE_STREAMER_H) \
+ $(TREE_H) $(CXX_PPH_H) cp/pph-streamer.h
cp/lex.o: cp/lex.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) \
$(C_PRAGMA_H) output.h input.h cp/operators.def $(TM_P_H) \
@@ -281,7 +280,7 @@ cp/decl.o: cp/decl.c $(CXX_TREE_H) $(TM_
cp/operators.def $(TM_P_H) $(TREE_INLINE_H) $(DIAGNOSTIC_H) $(C_PRAGMA_H) \
debug.h gt-cp-decl.h $(TIMEVAR_H) $(TARGET_H) $(PLUGIN_H) \
intl.h tree-iterator.h pointer-set.h $(SPLAY_TREE_H) \
- c-family/c-objc.h $(CXX_PPH_STREAMER_H)
+ c-family/c-objc.h $(CXX_PPH_H)
cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) cp/decl.h \
output.h toplev.h $(C_COMMON_H) gt-cp-decl2.h $(CGRAPH_H) \
$(C_PRAGMA_H) $(TREE_DUMP_H) intl.h $(TARGET_H) $(GIMPLE_H) pointer-set.h \
@@ -301,7 +300,7 @@ cp/class.o: cp/class.c $(CXX_TREE_H) $(T
$(SPLAY_TREE_H) pointer-set.h
cp/call.o: cp/call.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) toplev.h \
$(DIAGNOSTIC_CORE_H) intl.h gt-cp-call.h convert.h $(TARGET_H) langhooks.h \
- $(TIMEVAR_H) $(CXX_PPH_H) c-family/c-objc.h
+ $(TIMEVAR_H) c-family/c-objc.h
cp/friend.o: cp/friend.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H)
cp/init.o: cp/init.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) \
$(EXCEPT_H) $(TARGET_H)
@@ -332,7 +331,7 @@ cp/repo.o: cp/repo.c $(CXX_TREE_H) $(TM_
cp/semantics.o: cp/semantics.c $(CXX_TREE_H) $(TM_H) toplev.h \
$(FLAGS_H) output.h $(RTL_H) $(TIMEVAR_H) \
$(TREE_INLINE_H) $(CGRAPH_H) $(TARGET_H) $(C_COMMON_H) $(GIMPLE_H
[pph] More DECL merging. (issue5268042)
Use the mangled name for merging, as this should enable us to
handle function overloads. We use the regular identifier for other
declarations, as that should be sufficient and avoids the problem of
different typedefs mangling to the same name.
Merge struct members as well as namespace members. This will
eventually help with member declaration versus definition issues.
Change test cases to reflect the above.
Comment on other failing tests.
Comment on failing cache handling for merge.
Tested on x64.
Index: gcc/testsuite/ChangeLog.pph
2011-10-12 Lawrence Crowl cr...@google.com
* g++.dg/pph/p2pr36533.cc: Mark expected fail on unexpanded intrinsic.
* g++.dg/pph/p4pr36533.cc: Likewise.
* g++.dg/pph/p4mean.cc: Likewise.
* g++.dg/pph/c3variables.cc: Comment on reason for fail.
* g++.dg/pph/c4vardef.cc: Likewise.
Index: gcc/cp/ChangeLog.pph
2011-10-12 Lawrence Crowl cr...@google.com
* pph-streamer.h (pph_merge_name): New.
* pph-streamer.c (pph_merge_name): New.
* pph-streamer-out.c (pph_out_mergeable_tree_vec): Emit the vector
in declaration order.
(pph_out_merge_name): New.
(pph_write_any_tree): Use pph_out_merge_name instead of raw code.
* pph-streamer-in.c (pph_match_to_link): Use pph_merge_name.
(pph_in_binding_level): Also merge members of structs.
(pph_read_any_tree): Save read tree to determine if it is different
from the tree to be used.
Index: gcc/testsuite/g++.dg/pph/p2pr36533.cc
===
--- gcc/testsuite/g++.dg/pph/p2pr36533.cc (revision 179880)
+++ gcc/testsuite/g++.dg/pph/p2pr36533.cc (working copy)
@@ -1,2 +1,6 @@
/* { dg-options -w -fpermissive } */
+// pph asm xdiff 25347
+// xfail BOGUS INTRINSIC
+// failing to recognise memset as an intrinsic
+
#include p1pr36533.h
Index: gcc/testsuite/g++.dg/pph/c3variables.cc
===
--- gcc/testsuite/g++.dg/pph/c3variables.cc (revision 179880)
+++ gcc/testsuite/g++.dg/pph/c3variables.cc (working copy)
@@ -1,4 +1,5 @@
// pph asm xdiff 34997
+// xfail BOGUS DUPVAR
// tentative definition emitted twice
#include c0variables1.h
Index: gcc/testsuite/g++.dg/pph/p4mean.cc
===
--- gcc/testsuite/g++.dg/pph/p4mean.cc (revision 179880)
+++ gcc/testsuite/g++.dg/pph/p4mean.cc (working copy)
@@ -1,4 +1,8 @@
/* { dg-options -w -fpermissive } */
+// pph asm xdiff 39234
+// xfail BOGUS INTRINSIC
+// failing to recognize sqrt as an intrinsic
+
#include stdlib.h
#include stdio.h
#include math.h
Index: gcc/testsuite/g++.dg/pph/p4pr36533.cc
===
--- gcc/testsuite/g++.dg/pph/p4pr36533.cc (revision 179880)
+++ gcc/testsuite/g++.dg/pph/p4pr36533.cc (working copy)
@@ -1,2 +1,6 @@
/* { dg-options -w -fpermissive } */
+// pph asm xdiff 25347
+// xfail BOGUS INTRINSIC
+// failing to recognise memset as an intrinsic
+
#include p4pr36533.h
Index: gcc/testsuite/g++.dg/pph/c4vardef.cc
===
--- gcc/testsuite/g++.dg/pph/c4vardef.cc(revision 179880)
+++ gcc/testsuite/g++.dg/pph/c4vardef.cc(working copy)
@@ -1,4 +1,6 @@
// pph asm xdiff 00553
+// xfail BOGUS DUPVAR
+// definition emitted twice
#include c0vardef1.h
#include c0vardef2.h
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179880)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -803,7 +803,7 @@ pph_match_to_function (tree expr ATTRIBU
against an LINK of a chain. */
static tree
-pph_match_to_link (tree expr, location_t where, const char *idstr, tree* link)
+pph_match_to_link (tree expr, location_t where, const char *idstr, tree *link)
{
enum tree_code link_code, expr_code;
tree idtree;
@@ -817,7 +817,7 @@ pph_match_to_link (tree expr, location_t
if (link_code != expr_code)
return NULL;
- idtree = DECL_NAME (*link);
+ idtree = pph_merge_name (*link);
if (!idtree)
return NULL;
@@ -1072,7 +1072,7 @@ pph_in_binding_level (cp_binding_level *
*out_field = bl;
entity = bl-this_entity = pph_in_tree (stream);
- if (NAMESPACE_SCOPE_P (entity))
+ if (NAMESPACE_SCOPE_P (entity) || DECL_CLASS_SCOPE_P (entity))
{
if (flag_pph_debug = 3)
debug_tree_chain (bl-names);
@@ -1962,7 +1962,8 @@ pph_read_any_tree (pph_stream *stream, t
{
struct lto_input_block *ib = stream-encoder.r.ib;
struct data_in *data_in = stream-encoder.r.data_in;
- tree expr = NULL_TREE;
+ tree read = NULL;
+ tree expr = NULL;
enum pph_record_marker marker;
unsigned image_ix, ix;
enum LTO_tags tag;
@@ -1998,9 +1999,11 @@ pph_read_any_tree (pph_stream *stream, t
/* Materialize a new node
[pph] Rename functions to match convention. (issue5269041)
Rename some read/write functions to match current in/out conventions.
Specifically, pph_write_any_tree, pph_write_mergeable_links,
pph_write_tree_body, pph_write_tree_header, pph_read_any_tree,
pph_read_tree_body, and pph_read_tree_header change to their out/in
names.
Index: gcc/cp/ChangeLog.pph
2011-10-12 Lawrence Crowl cr...@google.com
* pph-streamer-out.c (pph_write_any_tree): Rename to pph_out_any_tree.
(pph_write_mergeable_links): Rename to pph_out_mergeable_links.
(pph_write_tree_body): Rename to pph_out_tree_body.
(pph_write_tree_header): Rename to pph_out_tree_header.
* pph-streamer-in.c (pph_read_any_tree): Rename to pph_in_any_tree.
(pph_read_tree_body): Rename to pph_in_tree_body.
(pph_read_tree_header): Rename to pph_in_tree_header.
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179886)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -513,14 +513,14 @@ pph_in_start_record (pph_stream *stream,
/* The core tree reader is defined much later. */
-static tree pph_read_any_tree (pph_stream *stream, tree *chain);
+static tree pph_in_any_tree (pph_stream *stream, tree *chain);
/* Load an AST from STREAM. Return the corresponding tree. */
tree
pph_in_tree (pph_stream *stream)
{
- tree t = pph_read_any_tree (stream, NULL);
+ tree t = pph_in_any_tree (stream, NULL);
return t;
}
@@ -530,7 +530,7 @@ pph_in_tree (pph_stream *stream)
static void
pph_in_mergeable_tree (pph_stream *stream, tree *chain)
{
- pph_read_any_tree (stream, chain);
+ pph_in_any_tree (stream, chain);
}
@@ -543,7 +543,7 @@ pph_read_tree (struct lto_input_block *i
{
/* Find data. */
pph_stream *stream = (pph_stream *) root_data_in-sdata;
- return pph_read_any_tree (stream, NULL);
+ return pph_in_any_tree (stream, NULL);
}
@@ -1710,7 +1710,7 @@ pph_in_tcc_declaration (pph_stream *stre
/* Read the body fields of EXPR from STREAM. */
static void
-pph_read_tree_body (pph_stream *stream, tree expr)
+pph_in_tree_body (pph_stream *stream, tree expr)
{
struct lto_input_block *ib = stream-encoder.r.ib;
struct data_in *data_in = stream-encoder.r.data_in;
@@ -1926,7 +1926,7 @@ pph_unpack_value_fields (struct bitpack_
Return the new tree. */
static tree
-pph_read_tree_header (pph_stream *stream, enum LTO_tags tag)
+pph_in_tree_header (pph_stream *stream, enum LTO_tags tag)
{
struct lto_input_block *ib = stream-encoder.r.ib;
struct data_in *data_in = stream-encoder.r.data_in;
@@ -1958,7 +1958,7 @@ pph_read_tree_header (pph_stream *stream
the tree may be unified with an existing tree in that namespace. */
static tree
-pph_read_any_tree (pph_stream *stream, tree *chain)
+pph_in_any_tree (pph_stream *stream, tree *chain)
{
struct lto_input_block *ib = stream-encoder.r.ib;
struct data_in *data_in = stream-encoder.r.data_in;
@@ -1999,7 +1999,7 @@ pph_read_any_tree (pph_stream *stream, t
/* Materialize a new node from IB. This will also read all the
language-independent bitfields for the new tree. */
- expr = read = pph_read_tree_header (stream, tag);
+ expr = read = pph_in_tree_header (stream, tag);
gcc_assert (read != NULL);
if (chain)
expr = pph_merge_into_chain (stream, expr, chain);
@@ -2026,7 +2026,7 @@ pph_read_any_tree (pph_stream *stream, t
circular references and references from children nodes. */
/* FIXME pph: We should not insert when read == expr, but it fails. */
pph_cache_insert_at (stream-cache, expr, ix, pph_tree_code_to_tag (expr));
- pph_read_tree_body (stream, expr);
+ pph_in_tree_body (stream, expr);
pph_new_trace_tree (stream, expr, chain != NULL);
Index: gcc/cp/pph-streamer-out.c
===
--- gcc/cp/pph-streamer-out.c (revision 179886)
+++ gcc/cp/pph-streamer-out.c (working copy)
@@ -620,7 +620,7 @@ pph_out_start_tree_record (pph_stream *s
/* The core tree writer is defined much later. */
-static void pph_write_any_tree (pph_stream *stream, tree t, bool mergeable);
+static void pph_out_any_tree (pph_stream *stream, tree t, bool mergeable);
/* Output non-mergeable AST T to STREAM */
@@ -628,7 +628,7 @@ static void pph_write_any_tree (pph_stre
void
pph_out_tree (pph_stream *stream, tree t)
{
- pph_write_any_tree (stream, t, false);
+ pph_out_any_tree (stream, t, false);
}
@@ -637,7 +637,7 @@ pph_out_tree (pph_stream *stream, tree t
static void
pph_out_mergeable_tree (pph_stream *stream, tree t)
{
- pph_write_any_tree (stream, t, true);
+ pph_out_any_tree (stream, t, true);
}
@@ -648,7 +648,7 @@ void
pph_write_tree (struct output_block *ob, tree expr, bool ref_p
ATTRIBUTE_UNUSED)
{
pph_stream *stream = (pph_stream *) ob-sdata;
- pph_write_any_tree (stream, expr, false);
+ pph_out_any_tree (stream, expr
[pph] Remove old tracing. (issue5271041)
Remove old trace facility. Having done so, rename pph_new_trace_tree
to pph_trace_tree and remove its unused stream parameter. Adjust
callers to match.
Bootstrap and PPH tests.
Index: gcc/cp/ChangeLog.pph
2011-10-12 Lawrence Crowl cr...@google.com
* pph-streamer.h (enum pph_trace_type): Remove.
(pph_trace): Likewise.
(pph_trace_tree): Likewise.
(pph_trace_uint): Likewise.
(pph_trace_bytes): Likewise.
(pph_trace_string): Likewise.
(pph_trace_string_with_length): Likewise.
(pph_trace_location): Likewise.
(pph_trace_chain): Likewise.
(pph_trace_bitpack): Likewise.
(pph_new_trace_tree): Rename pph_trace_tree.
Remove unused stream parameter.
* pph-streamer.c (enum pph_trace_type): Remove.
(pph_trace): Likewise.
(pph_trace_tree): Likewise.
(pph_trace_uint): Likewise.
(pph_trace_bytes): Likewise.
(pph_trace_string): Likewise.
(pph_trace_string_with_length): Likewise.
(pph_trace_location): Likewise.
(pph_trace_chain): Likewise.
(pph_trace_bitpack): Likewise.
(pph_new_trace_tree): Rename pph_trace_tree.
Remove unused stream parameter.
* pph-streamer-out.c (pph_out_any_tree): Call new pph_trace_tree
instead of old pph_new_trace_tree.
* pph-streamer-in.c (pph_in_any_tree): Call new pph_trace_tree
instead of old pph_new_trace_tree.
(pph_in_location): Change lead comment.
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179887)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -577,9 +577,9 @@ pph_read_location (struct lto_input_bloc
/* Read and return a location_t from STREAM.
- FIXME pph: If pph_trace didn't depend on STREAM, we could avoid having to
- call this function, only for it to call lto_input_location, which calls the
- streamer hook back to pph_read_location. */
+ FIXME pph: Tracing doesn't depend on STREAM any more. We could avoid having
+ to call this function, only for it to call lto_input_location, which calls
+ the streamer hook back to pph_read_location. Say what? */
location_t
pph_in_location (pph_stream *stream)
@@ -2028,7 +2028,7 @@ pph_in_any_tree (pph_stream *stream, tre
pph_cache_insert_at (stream-cache, expr, ix, pph_tree_code_to_tag (expr));
pph_in_tree_body (stream, expr);
- pph_new_trace_tree (stream, expr, chain != NULL);
+ pph_trace_tree (expr, chain != NULL);
/* If needed, sign the recently materialized tree to detect
mutations. Note that we only need to compute signatures
Index: gcc/cp/pph-streamer.c
===
--- gcc/cp/pph-streamer.c (revision 179886)
+++ gcc/cp/pph-streamer.c (working copy)
@@ -285,137 +285,10 @@ pph_add_include (pph_stream *stream, pph
}
-/* Data types supported by the PPH tracer. */
-enum pph_trace_type
-{
-PPH_TRACE_TREE,
-PPH_TRACE_UINT,
-PPH_TRACE_BYTES,
-PPH_TRACE_STRING,
-PPH_TRACE_LOCATION,
-PPH_TRACE_CHAIN,
-PPH_TRACE_BITPACK
-};
-
-
-/* Print tracing information for STREAM on pph_logfile. DATA is the
- memory area to display, SIZE is the number of bytes to print, TYPE
- is the kind of data to print. */
-
-static void
-pph_trace (pph_stream *stream, const void *data, unsigned int nbytes,
- enum pph_trace_type type)
-{
- const char *op = (stream-write_p) ? : ;
- const char *type_s[] = { tree, ref, uint, bytes, string, chain,
- bitpack };
-
- if ((type == PPH_TRACE_TREE || type == PPH_TRACE_CHAIN)
- !data
- flag_pph_tracer = 3)
-return;
-
- fprintf (pph_logfile, PPH: %s %s %s/%u,
- stream-name, op, type_s[type], (unsigned) nbytes);
-
- switch (type)
-{
-case PPH_TRACE_TREE:
- {
- const_tree t = (const_tree) data;
- if (t)
- {
-enum tree_code code = TREE_CODE (t);
-fprintf (pph_logfile, , code=%s, pph_tree_code_text (code));
-if (DECL_P (t))
- {
-fprintf (pph_logfile, , value=);
-#if 1
-print_generic_decl (pph_logfile,
-CONST_CAST (union tree_node *, t), 0);
-#else
-pph_dump_tree_name (pph_logfile, t, 0);
-#endif
- }
- }
- else
- fprintf (pph_logfile, , NULL_TREE);
- }
- break;
-
-case PPH_TRACE_UINT:
- {
- unsigned int val = *((const unsigned int *) data);
- fprintf (pph_logfile, , value=%u (0x%x), val, val);
- }
- break;
-
-case PPH_TRACE_BYTES:
- {
- size_t i;
- const char *buffer = (const char *) data;
-fprintf (pph_logfile, , value=);
- for (i = 0; i MIN (nbytes, 100); i++)
- {
- if (ISPRINT (buffer[i
[pph] Triage test status. (issue5271044)
Mark test x3hardorder.cc as passing. Update many other tests to
indicate their current failure reason. Fix the readme.
Index: gcc/testsuite/ChangeLog.pph
2011-10-13 Lawrence Crowl cr...@google.com
* g++.dg/pph/README: Put z files in regular expression.
* g++.dg/pph/x3hardorder.cc: Mark passing.
* g++.dg/pph/c1limits-externalid.cc: Add triage comment.
* g++.dg/pph/e4variables.cc: Likewise.
* g++.dg/pph/x1tmplclass1.cc: Likewise.
* g++.dg/pph/x1tmplclass2.cc: Likewise.
* g++.dg/pph/x4keyed.cc: Likewise.
* g++.dg/pph/x4keyex.cc: Likewise.
* g++.dg/pph/x4keyno.cc: Likewise.
* g++.dg/pph/x4resolve1.cc: Likewise.
* g++.dg/pph/x4resolve2.cc: Likewise.
* g++.dg/pph/x4structover1.cc: Likewise.
* g++.dg/pph/x4tmplclass2.cc: Likewise.
* g++.dg/pph/x4tmplfuncinln.cc: Likewise.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/x6dynarray3.cc: Likewise.
* g++.dg/pph/x6dynarray4.cc: Likewise.
* g++.dg/pph/x6rtti.cc: Likewise.
* g++.dg/pph/x7dynarray5.cc: Likewise.
* g++.dg/pph/x7rtti.cc: Likewise.
* g++.dg/pph/z4nontrivinit.cc: Likewise.
* g++.dg/pph/z4tmplclass1.cc: Likewise.
* g++.dg/pph/z4tmplclass2.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
Index: gcc/testsuite/g++.dg/pph/x4resolve1.cc
===
--- gcc/testsuite/g++.dg/pph/x4resolve1.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x4resolve1.cc (working copy)
@@ -1,4 +1,6 @@
// pph asm xwant 03374
+// This test produces overload differences because the declaration and
+// call orders are different between pph and textual parsing.
#include x0resolve1.h
#include x0resolve2.h
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (working copy)
@@ -1,4 +1,6 @@
// pph asm xdiff 37887
+// xfail BOGUS DIFF LABEL
+
#include x0tmplfuncninl1.h
#include x0tmplfuncninl2.h
#include a0tmplfuncninl_u.h
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (working copy)
@@ -1,4 +1,6 @@
// pph asm xdiff 05125
+// xfail BOGUS DUPFUN
+
#include x0tmplfuncninl3.h
#include x0tmplfuncninl4.h
#include a0tmplfuncninl_u.h
Index: gcc/testsuite/g++.dg/pph/x6dynarray3.cc
===
--- gcc/testsuite/g++.dg/pph/x6dynarray3.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x6dynarray3.cc (working copy)
@@ -1,5 +1,7 @@
// pph asm xdiff 30893
-// .Lnn labels emitted with different values of 'nn'.
+// xfail BOGUS UNKNOWN
+// Some branches seem to be missing.
+
#include x5dynarray3.h
#include a0integer.h
Index: gcc/testsuite/g++.dg/pph/x4keyno.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyno.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x4keyno.cc (working copy)
@@ -1,5 +1,6 @@
// { dg-xfail-if BOGUS MERGE AUXVAR { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus x4keyno.cc:11:1: error: redefinition of 'const char _ZTS5keyno
{ xfail *-*-* } 0 }
+// { dg-bogus x4keyno.cc:12:1: error: redefinition of 'const char _ZTS5keyno
{ xfail *-*-* } 0 }
+// The variable for the typeinfo name for 'keyno' is duplicated.
#include x0keyno1.h
#include x0keyno2.h
Index: gcc/testsuite/g++.dg/pph/x7dynarray5.cc
===
--- gcc/testsuite/g++.dg/pph/x7dynarray5.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x7dynarray5.cc (working copy)
@@ -1,4 +1,5 @@
-// { dg-xfail-if BOGUS { *-*-* } { -fpph-map=pph.map } }
+// { dg-xfail-if BOGUS POSSIBLY DROPPING SYMBOLS { *-*-* } {
-fpph-map=pph.map } }
+
#include x0dynarray4.h
#include x6dynarray5.h
Index: gcc/testsuite/g++.dg/pph/README
===
--- gcc/testsuite/g++.dg/pph/README (revision 179942)
+++ gcc/testsuite/g++.dg/pph/README (working copy)
@@ -1,7 +1,7 @@
The test names have the following convention on the prefix of their
names.
-[acdpxy][0-9]*
+[acdpxyz][0-9]*
a - auxillary headers
c - positive tests for C-level headers and sources
Index: gcc/testsuite/g++.dg/pph/x4resolve2.cc
===
--- gcc/testsuite/g++.dg/pph/x4resolve2.cc (revision 179942)
+++ gcc/testsuite/g++.dg/pph/x4resolve2.cc (working copy)
@@ -1,4 +1,6 @@
// pph asm xwant 37643
+// This test produces overload differences because
[pph] Fix merging for namespaces. (issue5280048)
Factor pph_out_binding_level and pph_in_binding_level into three
routines each: one to handle bindings that merge; one to handle those
that do not; and one to handle the parts common to both.
Streaming the global namespace now uses the mergeable version of
binding level streaming. At present we are merging only the global
namespace. Subsequent patches will merge nested namespaces and
classes.
Tested on x64.
Index: gcc/testsuite/ChangeLog.pph
2011-10-14 Lawrence Crowl cr...@google.com
* g++.dg/pph/c3variables.cc: Mark fixed.
* g++.dg/pph/c4vardef.cc: Mark fixed.
* g++.dg/pph/e4variables.cc: Mark fixed.
* g++.dg/pph/z4nontrivinit.cc: Mark with fewer failures.
Index: gcc/cp/ChangeLog.pph
2011-10-14 Lawrence Crowl cr...@google.com
* pph-streamer.h (extern pph_dump_binding): New.
* pph.c (pph_dump_binding): New.
(pph_dump_namespace): Call pph_dump_binding instead of inlining it.
* pph-streamer-in.c (pph_in_binding_level): Return to original value
return. Modify callers to match.
(pph_in_binding_level_1): New.
(pph_in_mergeable_binding_level): New.
(pph_decl_already_emitted): New.
(pph_in_symtab): Filter emmission by pph_decl_already_emitted.
(pph_in_identifiers): Fix comment.
(pph_in_scope_chain): Remove.
(pph_read_file_1): Call pph_in_mergeable_binding_level instead of
pph_in_scope_chain.
* pph-streamer-out.c (pph_out_binding_level_1): Handle only the parts
of bindings that do not differ with merging.
(pph_out_binding_level): Handle non-merging bindings.
(pph_out_mergeable_binding_level): New.
(pph_out_scope_chain): Rename to pph_out_global_binding. Call
pph_out_mergeable_binding_level instead of pph_out_binding_level_1.
Index: gcc/testsuite/g++.dg/pph/c3variables.cc
===
--- gcc/testsuite/g++.dg/pph/c3variables.cc (revision 179995)
+++ gcc/testsuite/g++.dg/pph/c3variables.cc (working copy)
@@ -1,7 +1,3 @@
-// pph asm xdiff 34997
-// xfail BOGUS DUPVAR
-// tentative definition emitted twice
-
#include c0variables1.h
#include c0variables2.h
#include a0variables2.h
Index: gcc/testsuite/g++.dg/pph/z4nontrivinit.cc
===
--- gcc/testsuite/g++.dg/pph/z4nontrivinit.cc (revision 179995)
+++ gcc/testsuite/g++.dg/pph/z4nontrivinit.cc (working copy)
@@ -1,5 +1,5 @@
-// pph asm xdiff 65039
-// xfail BOGUS DUPVAR
+// pph asm xdiff 46966
+// xfail BOGUS DOUBLE DYNINIT
#include x0nontrivinit1.h
#include x0nontrivinit2.h
Index: gcc/testsuite/g++.dg/pph/e4variables.cc
===
--- gcc/testsuite/g++.dg/pph/e4variables.cc (revision 179995)
+++ gcc/testsuite/g++.dg/pph/e4variables.cc (working copy)
@@ -1,6 +1,3 @@
-// pph asm xdiff 26015
-// xfail BOGUS DUPVAR
-
#include c0variables3.h
#include c0variables4.h
Index: gcc/testsuite/g++.dg/pph/c4vardef.cc
===
--- gcc/testsuite/g++.dg/pph/c4vardef.cc(revision 179995)
+++ gcc/testsuite/g++.dg/pph/c4vardef.cc(working copy)
@@ -1,6 +1,2 @@
-// pph asm xdiff 00553
-// xfail BOGUS DUPVAR
-// definition emitted twice
-
#include c0vardef1.h
#include c0vardef2.h
Index: gcc/cp/pph.c
===
--- gcc/cp/pph.c(revision 179995)
+++ gcc/cp/pph.c(working copy)
@@ -111,13 +111,21 @@ pph_dump_tree_name (FILE *file, tree t,
void
pph_dump_namespace (FILE *file, tree ns)
{
- cp_binding_level *level;
- tree t, chain;
- level = NAMESPACE_LEVEL (ns);
-
fprintf (file, namespace );
- print_generic_expr (file, ns, 0);
+ pph_dump_tree_name (file, ns, 0);
fprintf (file, {\n);
+ pph_dump_binding (file, NAMESPACE_LEVEL (ns));
+ fprintf (file, }\n);
+}
+
+
+/* Dump cp_binding_level LEVEL for PPH. */
+
+void
+pph_dump_binding (FILE *file, cp_binding_level *level)
+{
+ tree t, chain;
+
for (t = level-names; t; t = chain)
{
chain = DECL_CHAIN (t);
@@ -130,7 +138,6 @@ pph_dump_namespace (FILE *file, tree ns)
if (!DECL_IS_BUILTIN (t))
pph_dump_namespace (file, t);
}
- fprintf (file, }\n);
}
Index: gcc/cp/pph-streamer-in.c
===
--- gcc/cp/pph-streamer-in.c(revision 179995)
+++ gcc/cp/pph-streamer-in.c(working copy)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
#include cppbuiltin.h
#include toplev.h
#include parser.h
+#include pointer-set.h
typedef char *char_p;
DEF_VEC_P(char_p);
@@ -57,6 +58,7 @@ static VEC(char_p,heap) *string_tables =
pph_cache_insert_at (CACHE, DATA, IX, TAG); \
} while (0)
+/* FIXME pph: Unneeded macro. */
/* Same as ALLOC_AND_REGISTER, but instead
[pph] Refactor Vectors and Chains (issue5263051)
Factor the vector and chain output routines to remove boolean control
parameters. The functions pph_out_tree_vec_1 and pph_out_chain_1 split
their conditional parts of their implementation into their use cases,
calling each other as needed.
pph_out_tree_vec - nothing special
pph_out_tree_vec_unchain - unchaining
pph_out_mergeable_tree_vec - merging, unchaining, reversing
pph_out_tree_vec_filtered - filtering
pph_out_chain - nothing special
pph_out_chain_filtered - filtering
pph_out_mergeable_chain_filtered - merging, unchaining, reversing, filtering
This change fixes an ordering bug, but now causes an ICE to surface,
which will be addressed later.
Tested on x64.
Index: gcc/testsuite/ChangeLog.pph
2011-10-16 Lawrence Crowl cr...@google.com
* g++.dg/pph/x4tmplfuncinln.cc: Change failure to ICE.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
Index: gcc/cp/ChangeLog.pph
2011-10-16 Lawrence Crowl cr...@google.com
* pph-streamer.h (pph_dump_chain): New.
* pph-streamer.c (pph_trace_tree): Filter expressions from trace.
Print tree code.
* pph.c (pph_dump_tree_name): Add print newline.
(pph_dump_namespace): Remove print space.
(pph_dump_binding): Factor out dumping a chain.
(pph_dump_chain): New.
* pph-streamer-in.c (pph_in_mergeable_binding_level): Use
pph_dump_chain.
(pph_read_file_1): Change location of namespace dump.
* pph-streamer-out.c (pph_tree_matches): Remove redundant test.
(pph_out_tree_vec_1): Removed.
(pph_out_tree_vec): Takes core implementation from pph_out_tree_vec_1.
(pph_out_tree_vec_unchain): New. Takes core implementation from
pph_out_tree_vec_1.
(pph_out_mergeable_tree_vec): Likewise.
(pph_out_tree_vec_filtered): Likewise.
(chain2vec_filter): Prevent null filter.
(chain2vec): New. Handles case above.
(pph_out_chain_1): Removed.
(pph_out_chain): Takes core implementation from pph_out_chain_1.
(pph_out_chain_filtered): New. Takes core implementation from
pph_out_chain_1.
(pph_out_mergeable_chain_filtered): Likewise.
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 37887
-// xfail BOGUS DIFF LABEL
+// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
+// { dg-excess-errors Template list problems }
#include x0tmplfuncninl1.h
#include x0tmplfuncninl2.h
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 05125
-// xfail BOGUS DUPFUN
+// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
+// { dg-excess-errors Template list problems }
#include x0tmplfuncninl3.h
#include x0tmplfuncninl4.h
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc
===
--- gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc (working copy)
@@ -1,6 +1,6 @@
-// pph asm xdiff 16845
-// xfail BOGUS DUPFUN
-// double function1double(double) is duplicated
+// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus a0tmplfuncinln_g.h:17:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
+// { dg-excess-errors Template list problems }
#include x0tmplfuncinln1.h
#include x0tmplfuncinln2.h
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc
===
--- gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 65129
-// xfail BOGUS DUPFUNC
+// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus a0tmplfuncinln_g.h:17:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
+// { dg-excess-errors Template list problems }
#include x0tmplfuncinln3.h
#include x0tmplfuncinln4.h
Index: gcc/cp/pph.c
===
--- gcc/cp/pph.c(revision 180048)
+++ gcc/cp/pph.c(working copy)
@@ -101,7 +101,10 @@ pph_dump_tree_name (FILE *file, tree t,
else if (EXPR_P (t))
fprintf (file, %s\n, expr_as_string (t, flags));
else
[pph] Function Merging (issue5278047)
Add function merging. First, let function mangled names match.
Second, overwrite existing struct functions for merged functions.
Third, add filtering for already emitted functions.
The result is ICEs in the call graph code.
Add tests for overload sets with more than one member.
Bootstrapped on x64.
2011-10-16 Lawrence Crowl cr...@google.com
Index: gcc/testsuite/ChangeLog.pph
* g++.dg/pph/c4inline.cc: Change to ICE in cgraph.
* g++.dg/pph/x1keyed.cc: Likewise.
* g++.dg/pph/x1keyno.cc: Likewise.
* g++.dg/pph/x4keyed.cc: Likewise.
* g++.dg/pph/x4keyex.cc: Likewise.
* g++.dg/pph/x4keyno.cc: Likewise.
* g++.dg/pph/x4tmplclass1.cc: Likewise.
* g++.dg/pph/x4tmplclass2.cc: Likewise.
* g++.dg/pph/x4tmplfuncinln.cc: Likewise.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/x6rtti.cc: Likewise.
* g++.dg/pph/x7rtti.cc: Likewise.
* g++.dg/pph/z4tmplclass1.cc: Likewise.
* g++.dg/pph/z4tmplclass2.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
* g++.dg/pph/x1tmplclass2.cc: Change to missing function.
Index: gcc/cp/ChangeLog.pph
2011-10-16 Lawrence Crowl cr...@google.com
* pph-streamer-in.c (pph_match_to_overload): Comment on overloading.
(pph_match_to_function): Allow functions to match for merging.
Comment on overloading.
(pph_match_to_link): Comment on overloading.
(pph_in_struct_function): Implement overwriting a struct function
when merging.
(pph_in_symtab): Add filtering for already emitted functions.
* pph-streamer.h (pph_trace_tree): Add a boolean parameter specifying
whether or not the tree was actually merged.
* pph-streamer.c (pph_trace_tree): Add a boolean parameter specifying
whether or not the tree was actually merged. Change output to
correspond. Update callers.
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (revision 180072)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (working copy)
@@ -1,6 +1,5 @@
-// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
-// { dg-excess-errors Template list problems }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x4tmplfuncninl.cc:1:0: internal compiler error: in
cgraph_create_node, at cgraph.c:502 { xfail *-*-* } 0 }
#include x0tmplfuncninl1.h
#include x0tmplfuncninl2.h
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
===
--- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (revision 180072)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (working copy)
@@ -1,6 +1,5 @@
-// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in
instantiate_decl { xfail *-*-* } 0 }
-// { dg-excess-errors Template list problems }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus z4tmplfuncninl.cc:1:0: internal compiler error: in
cgraph_create_node, at cgraph.c:502 { xfail *-*-* } 0 }
#include x0tmplfuncninl3.h
#include x0tmplfuncninl4.h
Index: gcc/testsuite/g++.dg/pph/x4keyno.cc
===
--- gcc/testsuite/g++.dg/pph/x4keyno.cc (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x4keyno.cc (working copy)
@@ -1,5 +1,8 @@
-// { dg-xfail-if BOGUS MERGE AUXVAR { *-*-* } { -fpph-map=pph.map } }
-// { dg-bogus x4keyno.cc:12:1: error: redefinition of 'const char _ZTS5keyno
{ xfail *-*-* } 0 }
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x4keyno.cc:15:1: internal compiler error: in
cgraph_analyze_functions, at cgraphunit.c:1193 { xfail *-*-* } 0 }
+// { dg-excess-errors typeinfo name duplicatd }
+
+// Previously.
// The variable for the typeinfo name for 'keyno' is duplicated.
#include x0keyno1.h
Index: gcc/testsuite/g++.dg/pph/x1keyed.cc
===
--- gcc/testsuite/g++.dg/pph/x1keyed.cc (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x1keyed.cc (working copy)
@@ -1,3 +1,6 @@
+// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } }
+// { dg-bogus x1keyed.cc:12:1: internal compiler error: in
cgraph_analyze_functions, at cgraphunit.c:1193 { xfail *-*-* } 0 }
+
#include x0keyed1.h
int keyed::key( int arg ) { return mix( field arg ); }
Index: gcc/testsuite/g++.dg/pph/x7rtti.cc
===
--- gcc/testsuite/g++.dg/pph/x7rtti.cc (revision 180071)
+++ gcc/testsuite/g++.dg/pph/x7rtti.cc (working copy)
@@ -1,18 +1,7 @@
// FIXME pph: This should be a { dg=do run
Re: [pph] Make libcpp symbol validation a warning (issue5235061)
On 10/20/11, Gabriel Charette gcharet...@gmail.com wrote:
I just thought about something..
Earlier I said that ALL line_table issues were resolved after this
patch (as it ignores the re-included headers that were guarded, as the
non-pph compiler does naturally).
One problem remains however, I'm pretty sure that re-included
non-pph'ed header's line_table entries are still showing up multiple
times (as the direct non-pph childs of a given pph_include have their
line_table entries copied one by one from the pph file).
I think we were talking about somehow remembering guards context in
which DECLs were declared and then ignoring DECLs streamed in if they
belong to a given header guard type that was previously seen in a
prior include using the same non-pph header, allowing us to ignore
those DECLs that are re-declared when they should have been guarded
out the second time.
I'm not sure whether there is machinery to handle non-pph re-includes
yet... but... at the very least, I'm pretty sure those non-pph entries
still show up multiple times in the line_table.
Now, we can't just remove/ignore those entries either... doing so
would alter the expected location offset (pph_loc_offset) applied to
all tokens streamed in directly from the pph header.
What we could potentially do is:
- ignore the repeated non-pph entry
- remember the number of locations this entry was supposed to take
(call that pph_loc_ignored_offset)
- then for DECLs imported after it we would then need an offset of
pph_loc_offset - pph_loc_ignored_offset, to compensate for the missing
entries in the line_table
The problem here obviously is that I don't think we have a way of
knowing which DECLs come before, inside, and after a given non-pph
header included in the parent pph header which we are currently
reading.
Furthermore, a DECL coming after the non-pph header could potentially
refer to something inside the ignored non-pph header and the
source_location of the referred token would now be invalid (although
that might already be fixed by the cache hit which would redirect that
token reference to the same token in the first included copy of that
same header which wasn't actually skipped as it was first and which is
valid)
On Tue, Oct 11, 2011 at 4:26 PM, Diego Novillo dnovi...@google.com wrote:
@@ -328,8 +327,6 @@ pph_in_line_table_and_includes (pph_stream *stream)
int entries_offset = line_table-used -
PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
enum pph_linetable_marker next_lt_marker = pph_in_linetable_marker
(stream);
- pph_reading_includes++;
-
for (first = true; next_lt_marker != PPH_LINETABLE_END;
next_lt_marker = pph_in_linetable_marker (stream))
{
@@ -373,19 +370,33 @@ pph_in_line_table_and_includes (pph_stream *stream)
else
lm-included_from += entries_offset;
Also, if we do ignore some non-pph entries, the included_from
calculation is going to need some trickier logic as well (it's fine
for the pph includes though as each child calculates it's own offset)
- gcc_assert (lm-included_from (int) line_table-used);
-
Also, I think this slipped in my previous comment, but I don't see how
this assert could trigger in the current code. If it did trigger
something was definitely wrong as it asserts the offseted
included_from is referring to an entry that is actually in the
line_table...
lm-start_location += pph_loc_offset;
I'm wondering if we shouldn't just whitelist the problematic cases
that we know about in the system/standard headers. It seems that all
others we could reasonably complain to the maintainers of the code.
--
Lawrence Crowl
Re: thoughts on libatomic
On 4/23/12, Andrew MacLeod amacl...@redhat.com wrote: On 04/23/2012 03:29 PM, Richard Henderson wrote: - load_n.c: - I'm concerned about the CAS on read-only mprotected pages? Why again do we think this is safe? Does the standard explicitly allow this? Or should we just use a lock in this case? Andrew, you had a bit of back-and-forth with someone about this. Can you dig that up? yes, this keeps coming up again and again I think you should take it up with Lawrence Crowl... He was the one, (along with some input from Jeffrey Yasskin) that concluded that it was OK to use it, even if it was just sort of shoehorned in so that we could get 16 byte lock free on the most common architecture... The earlier discussion was about volatile. Given the way the C++ standard works, I think the volatile issue really only affects programmers that want to access device registers with atomic operations. Machines are lots more complicated than they were when I accessed device registers, so I don't have a good feel for what the current expectations are. The standard is somewhat vague on volatile, so I think the CAS-based read is likely to be permitted. Even so, I have brougt up the issue with the C++ standard folks, and maybe we can get some more feedback here. it came down to something like the architecture manual entry for cmpxchg16 states that a store cycle may be added by the hardware under the covers, and as such, it is a part of the basic machine description, and therefore we might as well use the instruction even though we may be adding the store cycle ourselves sometimes I haven't found the actual communication for reference yet, I'll look again tomorrow. The standard says nothing about mprotect, so the implementation gets to define all the semantics. However, if the hardware gives a fault for the CAS but the programmer specified a simple read, the programmer may be a bit miffed. Even so, I don't think we have any other reasonable choice in the short term. We will probably need to wait for hardware to catch up. -- Lawrence Crowl
Turn check macros into functions. (issue6188088)
The gcc source uses several constructs that GDB does not understand.
This patch corrects some of them. It affects only compilers built
with ENABLE_TREE_CHECKING, and hence release compilers are unaffected.
In particular, I change the implementation of CHECK macros using
__extension__ into macros calling template inline functions. I do not
change the accessor macros themselves.
The effect is that it now possible to get useful responses to gdb
command like
(gdb) print DECL_FUNCTION_CODE (decl)
To make the feature work, you must define two macros within gdb.
(gdb) macro define __FILE__ gdb
(gdb) macro define __LINE__ 1
The reason for these definitions is that gdb does not expand the magic
preprocessor macros, and then cannot find what it thinks is a regular
symbol. (There is now a gdb issue for this problem.) I added these
definitions to gdbinit.in.
The inline functions would normally introduce additional lines in a
sequence of gdb 'step' commands. To prevent that behavior, I have
added the new 'skip' command for the tree.h file to gdbinit.in. The
'skip' command works with gdb 7.4 and later.
(The better solution to the stepping behavior is a new gdb command.
See http://sourceware.org/bugzilla/show_bug.cgi?id=12940.)
Tested on x86-64.
Index: gcc/ChangeLog.cxx-conversion
2012-05-15 Lawrence Crowl cr...@google.com
* tree.h (tree_check): New.
(TREE_CHECK): Use inline function above instead of __extension__.
(tree_not_check): New.
(TREE_NOT_CHECK): Use inline function above instead of __extension__.
(tree_check2): New.
(TREE_CHECK2): Use inline function above instead of __extension__.
(tree_not_check2): New.
(TREE_NOT_CHECK2): Use inline function above instead of __extension__.
(tree_check3): New.
(TREE_CHECK3): Use inline function above instead of __extension__.
(tree_not_check3): New.
(TREE_NOT_CHECK3): Use inline function above instead of __extension__.
(tree_check4): New.
(TREE_CHECK4): Use inline function above instead of __extension__.
(tree_not_check4): New.
(TREE_NOT_CHECK4): Use inline function above instead of __extension__.
(tree_check5): New.
(TREE_CHECK5): Use inline function above instead of __extension__.
(tree_not_check5): New.
(TREE_NOT_CHECK5): Use inline function above instead of __extension__.
(contains_struct_check): New.
(CONTAINS_STRUCT_CHECK): Use inline function above instead of
__extension__.
(tree_class_check): New.
(TREE_CLASS_CHECK): Use inline function above instead of __extension__.
(tree_range_check): New.
(TREE_RANGE_CHECK): Use inline function above instead of __extension__.
(omp_clause_subcode_check): New.
(OMP_CLAUSE_SUBCODE_CHECK): Use inline function above instead of
__extension__.
(omp_clause_range_check): New.
(OMP_CLAUSE_RANGE_CHECK): Use inline function above instead of
__extension__.
(expr_check): New.
(EXPR_CHECK): Use inline function above instead of __extension__.
(non_type_check): New.
(NON_TYPE_CHECK): Use inline function above instead of __extension__.
(tree_vec_elt_check): New.
(TREE_VEC_ELT_CHECK): Use inline function above instead of
__extension__.
(omp_clause_elt_check): New.
(OMP_CLAUSE_ELT_CHECK): Use inline function above instead of
__extension__.
(tree_operand_check): New.
(TREE_OPERAND_CHECK): Use inline function above instead of
__extension__.
(tree_operand_check_code): New.
(TREE_OPERAND_CHECK_CODE): Use inline function above instead of
__extension__.
(TREE_CHAIN): Simplify implementation.
(TREE_TYPE): Simplify implementation.
(tree_operand_length): Move for compilation dependences.
* gdbinit.in: (macro define __FILE__): New.
(macro define __LINE__): New.
(skip tree.h): New.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 187477)
+++ gcc/tree.h (working copy)
@@ -727,195 +727,80 @@ enum tree_node_structure_enum {
is accessed incorrectly. The macros die with a fatal error. */
#if defined ENABLE_TREE_CHECKING (GCC_VERSION = 2007)
-#define TREE_CHECK(T, CODE) __extension__ \
-({ __typeof (T) const __t = (T); \
-if (TREE_CODE (__t) != (CODE)) \
- tree_check_failed (__t, __FILE__, __LINE__, __FUNCTION__,\
-(CODE), 0);\
-__t; })
+#define TREE_CHECK(T, CODE) \
+(tree_check ((T), __FILE__, __LINE__, __FUNCTION__, (CODE)))
-#define TREE_NOT_CHECK(T, CODE) __extension__ \
-({ __typeof (T) const __t = (T
Re: Turn check macros into functions. (issue6188088)
On 5/16/12, Richard Guenther richard.guent...@gmail.com wrote: On May 16, 2012 Diego Novillo dnovi...@google.com wrote: On 12-05-16 09:00 , Richard Guenther wrote: On May 16, 2012 Diego Novillodnovi...@google.com wrote: On 12-05-16 05:41 , Richard Guenther wrote: What's the reason for templating these functions? They all take trees as parameter!? The check templates replicate the behavior of the check macros, which carefully return the type of their argument. In essence, the check macros were templates. True. I don't recall what Lawrence had in mind, but I remember that by using templates here, you don't need to deal with the mess of distinguishing tree from const_tree, and having to force constness out with ugly casts. Well, but you get no typesafety for it in return. You can simply provide a const_tree overload. There's less typing if you use the template variant. Not sure why you say there is less type safety with templates. Because it accepts any type as tree argument? It's of course not less type safety than using macros, but less type safety compared to not using templates. The overload works if the only types are tree and const_tree. In the future, we may wish to refine the types so that, for example, we can have a pointer to a decl and ask if it is a var decl. The implementation approach is to make small steps. It's a tradeoff between small steps that may show only incremental value, or big steps that risk rejection. In addition, merging is easier with small steps. With using templates you are also forced to retain these functions in the header file - another thing that I suppose you guys were about to fix? It's after all debugging code. No, templated functions must always stay in the header file. There is no changing that. If they ain't templates they are not templates. And thus do not need to stay in the header. Not sure what you are after here ;) Personally, I would like to get to the point where we take advantage of static typing and the vast bulk of these checks are eliminated. Additionally, templates are producing slightly smaller code than the non-template variant (about 0.2% smaller). I'm not actually sure why this happens, but it's consistent across all binaries. Well, what did you compare? Make sure to not have -fkeep-inline-functions, otherwise you get all bodies as compared to only the instantiated bodies. Two bootstrapped compilers built exactly the same, except one was using the template version, the other using the straight inline functions with const_tree parameters and CONST_CAST_TREE in return values. That's of course not exactly the same. The checking fns should be able to unconditionally use const_tree anyway. The two versions are inline templates functions versus macros. I expect the non-template version would be roughly the same size as the template version. -- Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
On 5/17/12, Tom Tromey tro...@redhat.com wrote: Lawrence == Lawrence Crowl cr...@google.com writes: Tom Doesn't this mean that if you have checking enabled, and you use Tom the wrong macro on some tree, cc1 will crash? That seems like Tom a distinct minus to me. Lawrence Yes, it does mean that, but it is a net overall improvement. It is a net debugging improvement compared to trunk, but not compared to using 'macro define's. If you know about them and your debuggers supports them. It was two years before I learned about those macros. By moving the source closer to standard code, we reduce the frustration for new developers. I was particularly frustrated by this problem. It would be nice if there were a way to make cc1 not crash due to user error in the debugger. I think this kind of crash will be especially crazy-making. Agreed. Intercept abort in gdb? Lawrence These extensions are not on gdb's list of things to Lawrence implement. As a digression - I guess they could be. That would certainly help in several when using gdb. It would not help if someone needs or wants to use a different debugger. Reusing the compiler for this seems like the only way to go. But, we did look at using g++ to parse C++ expressions from gdb, and it was too slow :-(. We're going to look again, at least to generate some bug reports if we can. It's a tough problem, particularly as C/C++ and their compilers were not designed for a read-eval-print-loop environment. -- Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
On 5/17/12, Mike Stump mikest...@comcast.net wrote:
On May 17, 2012, at 2:41 PM, Lawrence Crowl wrote:
Reusing the compiler for this seems like the only way to go.
But, we did look at using g++ to parse C++ expressions from gdb,
and it was too slow :-(. We're going to look again, at least to
generate some bug reports if we can.
It's a tough problem, particularly as C/C++ and their compilers
were not designed for a read-eval-print-loop environment.
This of course is trivial to do,
I do not think that word means what you think it means. :-)
and reasonably fast as well. We have an existence proof that
they can generate a call on the target system, all one needs to
do would be to locate the target compiler, compile up a strapping
routine, push a dlopen of that routine onto the target system and
then call dlsym to grab a handle to it. The strapping routine
can set up an environment and communication channels as needed.
If could even allocate a thread and have it always be live
(non-stop), if one wanted.
I agree that it is possible.
Oh, and the benefit, the language supported by the code closely
matches the compiler. Speed, 1/40 of a second, fast enough for
command like use. Want to evaluate it 100 times, sure, just
be sure to cache the dlopen/dlsym, and notice that you can do a
million invocations, well, I'd guess a million times faster than
gdb currently.
Agree.
If gdb won't solve these problem, dump it, move to lldb, I can
assure you, they will not fail to solve the problem, just because
gdb can't or won't. The name of the game is compete or die.
If you use the clang libraries, you can read eval, print... and
the performance would likely be even better. After all, it can
be used to do OpenCL style programming, and that certainly was
built for speed. So, I reject the idea this all isn't trivial.
Yes, clang was designed to handle this use case.
$ cat eval.c
#include stdio.h
#include stdlib.h
#include dlfcn.h
#include sys/time.h
int main() {
char buf[4096];
while (fgets (buf, sizeof (buf), stdin) != NULL) {
struct timeval tv1, tv2;
gettimeofday(tv1, 0);
FILE *fp = popen (gcc -w -xc - -shared -o test.so, w);
if (fputs (buf, fp) 0) {
printf (write fails\n);
exit (1);
}
if (pclose (fp) != 0) {
printf (compile fails\n);
exit (1);
}
void *vp = dlopen(test.so, RTLD_NOW|RTLD_LOCAL|RTLD_FIRST);
void *fcn = dlsym (vp, eval);
if (fcn) {
void (*func)();
func = fcn;
printf(about to run\n);
func();
gettimeofday(tv2, 0);
printf(done running, took %ld usecs\n, tv2.tv_sec*100
+ tv2.tv_usec - tv1.tv_sec*100 - tv1.tv_usec);
} else {
printf (eval not found\n);
}
if (dlclose (vp) 0) {
printf (dlclose fails\n);
exit (1);
}
}
}
$ gcc eval.c -o eval -O3
$ ./eval
int eval () { printf (hi\n); }
about to run
hi
done running, took 28419 usecs
int eval () { printf (hi\n); }
about to run
hi
done running, took 29300 usecs
int eval () { printf (hi\n); }
about to run
hi
done running, took 25755 usecs
int eval () { printf (hi\n); }
about to run
hi
done running, took 28295 usecs
I like the example, but it sidesteps the main problem, which is
putting the expression in the context from which it was called.
For instance if I stop in the middle of a function and type
print foo-bar( a + b )
I need to find the find the variable names, lookup their types,
do overload resolution, possibly do template instantiation, etc,
all as if the compiler was in the scope defined for the line that
the debugger is stopped at. That's the hard part.
Maybe your thinking of lisp, certainly that language makes
it hard. ;-P
Actually, a read-eval-print loop in Lisp is easy.
--
Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
On 5/18/12, Diego Novillo dnovi...@google.com wrote: So, I would like to figure out what to do with this. We have a usability problem wrt deubgging that I would like to fix. The only way we have of using all the tree accessor macros from GDB is to convert the checks into functions (converting the actual accessor macros would also work, but that's a different story). Now that we have the ability to skip functions in gdb, making tree checking into functions should not be a problem. Additionally, we can make gdb skip other common functions (like tree_code_length). We can do this in trunk today using a variant of Lawrence's original patch (http://gcc.gnu.org/ml/gcc-patches/2011-09/msg01649.html). This uses no C++ features, though it weakens type checking by removing away constness. The difference between then and now is that we now have the gdb skip command, which fixes the problem of introducing another step point into debugging code using the macros. In the cxx-conversion branch, we can use overloads, which will DTRT with const. My question is, what do folks prefer? a) The trunk patch today, using no C++ features. b) Wait for the cxx-conversion variant? Incidentally, making these expression statements into out-of-line functions saves about 3% in the final binary size, but increases compile time by about 10% (tested on several large .o files in bld/gcc), so we likely want to keep them as inline functions. With the inline functions, if the compiler is doing no inlining, we should still get near to that 3% reduction in binary size. -- Lawrence Crowl
Re: [cxx-conversion] Convert vec.[ch] to C++ [1/3] (issue6233044)
On 5/23/12, Diego Novillo dnovi...@google.com wrote: OK for cxx-conversion branch? LGTM. -- Lawrence Crowl
Re: [cxx-conversion] Convert vec.[ch] to C++ [2/3] (issue6236043)
On 5/23/12, Diego Novillo dnovi...@google.com wrote: Part 2 of the VEC C++ conversion. This patch implements the gengtype changes. LGTM. -- Lawrence Crowl
Re: [cxx-conversion] Convert vec.[ch] to C++ [3/3] (issue6236044)
On 5/23/12, Diego Novillo dnovi...@google.com wrote: Part 3 of the VEC C++ conversion. This patch implements all the client code changes needed by the API changes made by the first patch. LGTM. -- Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
On 5/21/12, Mike Stump mikest...@comcast.net wrote:
On May 18, 2012, at 7:48 PM, Lawrence Crowl wrote:
On 5/17/12, Mike Stump mikest...@comcast.net wrote:
On May 17, 2012, at 2:41 PM, Lawrence Crowl wrote:
Reusing the compiler for this seems like the only way to go.
But, we did look at using g++ to parse C++ expressions from gdb,
and it was too slow :-(. We're going to look again, at least to
generate some bug reports if we can.
It's a tough problem, particularly as C/C++ and their compilers
were not designed for a read-eval-print-loop environment.
This of course is trivial to do,
I do not think that word means what you think it means. :-)
Excellent reference... Yeah, well, just a little more work to do:
$ ./eval
extern C int printf(const char *...); int k2 = *(int*)4294971592;
void eval() { printf(k is %d\n, k2); }
about to run
k is 42
done running, took 32540 usecs
Notice, this sucked a int k2 = 42; that was a variable inside
the context in which I wanted a k2. The nature of the variable
is irrelevant, could be a stack variable, could be a global,
a static top-level, all I need to know, is where it is (the
address) and the type and I generate a binding stub for it into
the context, and presto, complete access to most all variables.
This does make use of C++ to manage the variables in a slightly
nicer way. One either has to inject all variables and functions
into the context into which the program fragment is compiled,
or notice which variables and function are or might be used and
inject just those.
I've seen ptype foo print the types of variables, so, it isn't
far off, and certainly gdb knows about the symbol table and can
figure out the addresses of those variables, right?!
For variables that are not optimized out of memory, I think this
can work. But if you need to go for registers, or undo a variable
elimination, the effort gets harder.
Now, certainly one can find additional issues, and yes, there
is a bit of work to get 100% reliability that we'd like from
the scheme. For example, packed structures would need work, as
ptype isn't going to get that right from that start. I think the
functionality one would get would be good enough to start with,
and in time, better support for things ptype doesn't get right
would fix a certain class of problems.
Agreed.
So, what another 1000 lines to generate the context for the
fragment... I guess finding references could be annoying, and the
time required to generate and compile the entire context could
be annoying...
Doing C++ would be harder, though, not quite for the reason
you gave.
I like the example, but it sidesteps the main problem, which is
putting the expression in the context from which it was called.
For instance if I stop in the middle of a function and type
print foo-bar( a + b )
I need to find the find the variable names, lookup their types,
ptype a does the lookup for a, and finds the type, so that isn't
hard, same with b and foo. Harder would be to synthesize the
context for the fragment to live in. To do this and work with
templates, you'd need the entire template bodies to be in the
debug information and to output them when generating the context
that might use them.
Yes, you essentially need to come close to sending the compiler's
symbol tables through the debug information. I don't know of any
compiler/debugger pair that is close to that capability. That said,
I don't know all such pairs either.
class A {
foo() { }
}
when stopped in foo, one needs to generate:
class A {
foo();
__eval() { fragment }
}
and then call __eval(lookup(this))... The compiler does
the overload resolution, the template instantiation and so on.
To ensure the compiler does name loookup right, one just needs
to generate the context for that fragment carefully.
Agreed.
do overload resolution, possibly do template instantiation, etc,
all as if the compiler was in the scope defined for the line that
the debugger is stopped at. That's the hard part.
I'm not saying that the task is impossible, only that there is a
lot of work to do before we get there.
--
Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
On 5/21/12, Tom Tromey tro...@redhat.com wrote: Alexander == Alexander Monakov amona...@ispras.ru writes: Alexander Hm, isn't GDB's 'set unwindonsignal on' enough to fix it? Alexander It's useful to have that in your .gdbinit anyway, because the Alexander same issue arises when calling debug_* functions in cc1 from Alexander the debugger. Yeah, why didn't I remember that? I think it should suffice. Thanks for the reminder. Should I add that to my patch to gdbinit.in? -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/24/12, Gabriel Dos Reis g...@integrable-solutions.net wrote: On May 24, 2012 Lawrence Crowl cr...@google.com wrote: Add a type-safe hash table, typed_htab. Uses of this table replace uses of libiberty's htab_t. The benefits include less boiler-plate code, full type safety, and improved performance. Lawrence, is there any chance you could just call it hash_table? After the conversion, we will be living most of the time in a typed world, so the typed_ prefix will be redundant if not confusing :-) I agree that the name is not great, and have no love of it. I'd like to get all the name suggestions on the table before I to a renaming pass. [...] The type-safe hash table is a template, taking the element type and the operational functions as template parameters. Passing the operational functions as template parameters, rather than function pointers, exposes the calls to inlining at -O2. A side effect is that declarations of hash tables move to after the declarations of those functions. A further side effect is that the control block shrinks from 108 bytes to 44 bytes. There is otherwise no effect on data size. Nice. Do you anticipate that at some point we could just pass function object classes as template arguments as opposed to functions? That of course would retain the nice property of inlining, but it would remove the need to make the function have an external linkage. The downside is that change would touch more places than your current patch does. Yes, we could do that. I'm a little reluctant to make too many steps in any one patch. As an example, we could move the control block into the variable for most uses of the table, but not all. Doing so would complicate this patch, and so I thought it best to make that change a separate issue. That said, I'll let y'all decide how much to put in any one piece. -- Lawrence Crowl
Re: [cxx-conversion] Convert vec.[ch] to C++ [1/3] (issue6233044)
On 5/24/12, Diego Novillo dnovi...@google.com wrote:
On 12-05-24 04:16 , Richard Guenther wrote:
On May 23, 2012 Diego Novillodnovi...@google.com wrote:
Some client code changes were needed:
1- The allocation names 'heap', 'stack' and 'gc' are not embedded in
the type name anymore. They are enum values used as a template
argument for functions like VEC_alloc() and VEC_free(). Since they
are now separate identifiers, some existing code that declared
variables with the names 'heap' and 'stack' had to change.
I did not change these enum values to better names because that
would have meant changing a lot more client code. I will change
this in a future patch.
Yeah. Can we have
template...
struct vec {
enum { gc, heap, stack };
...
}
and use vecT, vec::stack instead? Not sure if this or something
similar
is valid C++.
Sure. Something involving a new namespace, like Gaby suggested
down-thread.
I like namespaces, but using namespace does not work the way most
people think it does. So, I was planning to defer that issue for
a while.
2- VEC_index and VEC_last now return a reference to the element. The
existing APIs implemented two versions of these functions. One
returning the element itself (used for vec_tT *). Another
returning a pointer to the element (used for vec_tT).
This is impossible to represent in C++ directly, as functions
cannot be overloaded by their return value. Instead, I made them
return a reference to the element. This means that vectors storing
pointers (T *) do not need to change, but vectors of objects (T) do
need to change.
We have fewer vec_tT than vec_tT *, so this was the smaller
change (which still meant changing a few hundred call sites).
We still can have const overloads when taking a const vec though?
I'm going to say 'sure' because I don't really think I understand this
question :)
Yes, we can.
3- The family of VEC_*push and VEC_*replace functions have overloads
for handling 'T' and 'const T *' objects. The former is used for
vec_tT, the latter for vec_tT*. This means, however, than when
pushing the special value 0 on a vec_tT*, the compiler will not
know which overload we meant, as 0 matches both. In these cases
(not very many), a static cast is all we need to help it.
I'm not terribly content with this one. I'll try to have a better
approach in the second iteration (which will liberally change all
client code).
I wonder whether explicitely specifying the template type is better?
Thus, VEC_safe_pushT *, gc(0) instead of automatically deducing the
template argument?
Not sure. I didn't like the casts, Lawrence had another suggestion
involving something like C++11's nullptr. Lawrence, what was that
nullptr thing you were telling me earlier?
One would write VEC_save_push(v, nullptr), which unambiguously
means the null pointer. C++11 has nullptr built in to the language,
but we can approximate it with:
struct nullptr_t {
template typname T
operator T*() { return 0; }
} nullptr;
The downside is that using plain 0 would still be ambiguous.
Programmers that mean the integer would need to cast the 0.
4- I extended gengtype to understand templated types. These changes
are not as ugly as I thought they would be. Gengtype has some
hardwired knowledge of VEC_*, which I renamed to vec_t. I'm not
even sure why gengtype needs to care about vec_t in this way, but I
was looking for minimal changes, so I just did it.
The other change is more generic. It allows gengtype to deal with
templated types. There is a new function (filter_type_name) that
recognizes C++ special characters '','' and ':'. It turns them
into '_'. This way, gengtype can generate a valid identifier for
the pre-processor. It only does this in contexts where it needs a
pre-processor identifier. For everything else, it emits the type
directly. So, the functions emitted in gt-*.h files have proper
template type references.
Well, that part is of course what needs to change. I don't think we want
gengtype to work like this for templates as this does not scale.
Well, certainly, but that was outside the scope of this one patch. I'm
trying to make small incremental steps.
2- Change VEC_index into operator[].
3- Change VEC_replace(...) to assignments using operator[].
I think these should not be done for now - they merely add syntactic
sugar, no?
It makes the code more compact, readable and easier to write.
I can certainly wait until the branch has been merged to make
all these changes. It's going to affect a boatload of call sites.
How about picking a one or two files to show the effect, but leave
the bulk of the call site changes to later changes directly on trunk?
--
Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Diego Novillo dnovi...@google.com wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). Lawrence, have you had a chance to update them with your latest edits? Not yet; they're on the desk in front of me. I'll do that ASAP. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Paweł Sikora pl...@agmk.net wrote: On Friday 25 of May 2012 10:15:54 Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). on this page we can read: The following features of the C++ language should in general be avoided in GCC code. (...) Defining new templates (use of existing templates, e.g., from the standard library, is fine). That rule came from Ian's approach with gold. It works well for a new project. It does not work well in gcc. so, why you just don't use the hash table implementation from libstdc++? Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, so we would have to instrument the library. Second, the GTY parser doesn't handle C++, so we would have to redo the GTY parser. Third, PCH requires gc, which requires GTY, so we cannot easily get rid of gc. Fourth, any use of libstdc++ will make different tradeoffs in size and performance, so any switch would require both a change in form and a change in substance. We would simultaneously have to show benefit on two axes, which is a predictor of trouble. So, instead we chose to take a more incremental approach, exploiting C++ with the existing algorithms and requiring mostly mechanical changes in client code. Both the hash table and vector patches take that approach. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Michael Matz m...@suse.de wrote:
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+return htab-size;
+ }
(and various other places) - formatting is wrong, missing space
between (.
Yes, an omission that's a consequence of dealing with several
coding conventions.
And it doesn't start at the first column, and type isn't on a
separate line. I realize that this is a member method, hence
indenting and C GNU coding standards conflict, but the latter do
have some nice properties (for instance that 'grep ^func_name *.c'
finds only the definition, not all the calls to func_name).
We can address this issue by requiring all function definitions
to appear outside the class definition. The code is more verbose,
but livable.
I think we need a discussion about style, now that actually people
are working on this.
Yes.
+void
+typed_htab Element, Hash, Equal, Remove, Allocator
+::dispose ()
Do we want to format methods this way?
Don't know, but at least it starts at the first column, and
grepping for ^:: will give only C++ methods, 'grep -B1 ^::' even
including class. So it's no too horrible, despite the syntactic
C++ noise.
For template classes, the specification of an out-of-line member
function is going to be long, so a single-line specification is
likely to overflow often. Whatever we chose, it should be reasonably
extended to multiple lines.
--
Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Jakub Jelinek ja...@redhat.com wrote: On Fri, May 25, 2012 at 10:15:54AM -0400, Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). That page is quite inconsistent. E.g. it first talks about retaining space before ( (which I really hope will be retained, it makes code more readable), but in the example it doesn't bother with it. And the long line example is horribly ugly, ending line with ( ? That ( should surely go on the next line after the few spaces. It should list also examples of when the first parameter isn't too long, but there are too many parameters and so some wrapping is needed, etc. We will clean the page. There are technical guidelines that we can, and I think should, apply to the coding convention. In particular, I believe any coding convention should be consistent with the grammar of the language. Both the GNU style and the C++ Standard style break that guideline in different respects. For example, consider the following GNU code. int *p = *foo (3); Here, the convention implies that foo binds more tightly to the * than it does to the (. This implication is false. Now consider the C++ Standard equivalent. int* p = *foo(3); It fixes the binding of foo, but implies that * is syntactically closer to int than it is to p. That implication is false. Now consider a form that is consistent with the actual grammar of the language: int *p = *foo(3); Here there is no implication that foo binds more closely to either the * or the (, so the form is consistent with the grammar. We can go further and have the spacing reinforce the grammar, but lines will get longer. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/24/12, Jakub Jelinek ja...@redhat.com wrote:
On Thu, May 24, 2012 at 09:43:42AM -0700, Lawrence Crowl wrote:
Add a type-safe hash table, typed_htab. Uses of this table
replace uses of libiberty's htab_t. The benefits include less
boiler-plate code, full type safety, and improved performance.
You haven't looked at the most important problem of that approach
- code bloat.
Are you claiming that the size of the binary is more important than
run-time performance, type safety, and source code size?
hashtab.o .text is currently ~ 4KB on x86_64, in your version
only very small part out of it is shared. In a cc1plus binary
I count roughly 179 htab_create{,_ggc} calls, ignoring the first
parameter (initial size) that is roughly 139 unique hash/eq/del
fn combinations, thus you'll end up with over hundred copies of
most of the hashtab code,
First, the new hash table code is smaller because it avoid supporting
special cases that we are not using in practice. What was formerly
conditional is now unconditional, leading to tighter binaries for
a given function.
As you say, there will be more functions in the source, leading to
an increased executable size. Setting aside run-time performance,
why is that size important?
in many of the cases performance critical and thus its I-cache
friendliness is quite important.
It is precisely the performance critical code that needs the template
approach. The approach avoids two indirect calls, which go across
translation units. The optimizer has no ability to inline the often
trivial actions within those indirect calls. This problem is most
severe in the traverse functions, because the optimize cannot see
enough to do anything useful with the loop. The inliner also has no
ability to elide comparisons for null function pointers. In short,
with the libiberty code, the operations must span a large number
of completely useless operations. Those operations disappear with
the template approach. The result is that the working set in the
cache is much smaller.
That said, if you think code size is important for tables that
are not performance critical, I can provide a version of the code
that simply reuses libiberty and gets the type safety without
the performance. I don't have a good intuition for which of those
tables are performance-critical, so please give me an idea of which
they are.
--
Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Mike Stump mikest...@comcast.net wrote: On May 25, 2012, at 10:50 AM, Lawrence Crowl wrote: Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, If portability to other C++ compilers wasn't a concern, we could extend out g++ to make supporting GTY better, so that we can simplify and refine the GTY stuff. Personally, I would rather see if we can take advantage of C++ features to reduce garbage and then use the Boehm collector. There is too much manual management with GTY, and I'd rather the compiler leverage mainstream practice rather than depart from it. I fear we need some light weight reflection, might make a nice language feature for a future C++ standard, if done well. There has been some talk of such a language feature. As yet, the only concrete proposal does not have wide support. I suspect it will take several more proposals before we hit upon one that will get wide support. If you have any good ideas, now would be a really excellent time to speak up! -- Lawrence Crowl
[cxx-conversion] Update hash-table to new coding conventions. (issue6430066)
Change new C++ code to follow the new C++ coding conventions. This patch is
part one, which changes to out-of-line method definitions and does other minor
changes. Part two will fix the spacing problems. The two-part approach makes
diffs sensible.
Index: gcc/ChangeLog.cxx-conversion
2012-07-20 Lawrence Crowl cr...@google.com
* hash-table.h (xcallocator::control_alloc): Move definition
out of class.
(xcallocator::data_alloc): Likewise.
(xcallocator::control_free): Likewise.
(xcallocator::data_free): Likewise.
(struct hash_table): Use class keyword.
(hash_table::hash_table): Move definition out of line.
(hash_table::is_created): Likewise.
(hash_table::find): Likewise.
(hash_table::find_slot): Likewise.
(hash_table::remove_elt): Likewise.
(hash_table::size): Likewise.
(hash_table::elements): Likewise.
(hash_table::collisions): Likewise.
(hash_table::create): Put method name on same line as class qualifier.
(hash_table::dispose): Likewise.
(hash_table::expand): Likewise.
(hash_table::empty): Likewise.
Index: gcc/hash-table.h
===
--- gcc/hash-table.h(revision 189670)
+++ gcc/hash-table.h(working copy)
@@ -36,30 +36,43 @@ along with GCC; see the file COPYING3.
template typename Type
struct xcallocator
{
+ static Type *control_alloc (size_t count);
+ static Type *data_alloc (size_t count);
+ static void control_free (Type *memory);
+ static void data_free (Type *memory);
+};
+
/* Allocate memory for COUNT control blocks. */
- static Type *control_alloc (size_t count)
+template typename Type
+inline Type *
+xcallocator Type::control_alloc (size_t count)
{ return static_cast Type * (xcalloc (count, sizeof (Type))); }
-
+
/* Allocate memory for COUNT data blocks. */
- static Type *data_alloc (size_t count)
+template typename Type
+inline Type *
+xcallocator Type::data_alloc (size_t count)
{ return static_cast Type * (xcalloc (count, sizeof (Type))); }
/* Free memory for control blocks. */
- static void control_free (Type *memory)
+template typename Type
+inline void
+xcallocator Type::control_free (Type *memory)
{ return ::free (memory); }
+
/* Free memory for data blocks. */
- static void data_free (Type *memory)
+template typename Type
+inline void
+xcallocator Type::data_free (Type *memory)
{ return ::free (memory); }
-
-};
/* A common function for hashing a CANDIDATE typed pointer. */
@@ -182,27 +195,34 @@ template typename Element,
int (*Equal) (const Element *existing, const Element * candidate),
void (*Remove) (Element *retired),
template typename Type class Allocator = xcallocator
-struct hash_table
+class hash_table
{
private:
hash_table_control Element *htab;
-
Element **find_empty_slot_for_expand (hashval_t hash);
void expand ();
public:
+ hash_table ();
void create (size_t initial_slots);
+ bool is_created ();
void dispose ();
+ Element *find (Element *comparable);
Element *find_with_hash (Element *comparable, hashval_t hash);
+ Element **find_slot (Element *comparable, enum insert_option insert);
Element **find_slot_with_hash (Element *comparable, hashval_t hash,
enum insert_option insert);
void empty ();
void clear_slot (Element **slot);
+ void remove_elt (Element *comparable);
void remove_elt_with_hash (Element *comparable, hashval_t hash);
+ size_t size();
+ size_t elements();
+ double collisions();
template typename Argument,
int (*Callback) (Element **slot, Argument argument)
@@ -211,11 +231,18 @@ public:
template typename Argument,
int (*Callback) (Element **slot, Argument argument)
void traverse (Argument argument);
+};
/* Construct the hash table. The only useful operation next is create. */
- hash_table ()
+template typename Element,
+ hashval_t (*Hash) (const Element *candidate),
+ int (*Equal) (const Element *existing, const Element * candidate),
+ void (*Remove) (Element *retired),
+ template typename Type class Allocator
+inline
+hash_table Element, Hash, Equal, Remove, Allocator::hash_table ()
: htab (NULL)
{
}
@@ -223,7 +250,13 @@ public:
/* See if the table has been created, as opposed to constructed. */
- bool is_created ()
+template typename Element,
+ hashval_t (*Hash) (const Element *candidate),
+ int (*Equal) (const Element *existing, const Element * candidate),
+ void (*Remove) (Element *retired),
+ template typename Type class Allocator
+inline bool
+hash_table Element, Hash, Equal, Remove, Allocator::is_created ()
{
return htab != NULL;
}
@@ -231,7 +264,13 @@ public:
/* Like find_with_hash, but compute the hash value from
[cxx-conversion] Update hash-table to new coding conventions. (Part 2) (issue6435049)
Change new C++ code to follow the new C++ coding conventions. This patch is
part two, which changes the spacing for the bodies of methods formerly
defined in-class. The two-part approach makes diffs sensible.
Index: gcc/ChangeLog.cxx-conversion
2012-07-23 Lawrence Crowl cr...@google.com
* hash-table.h (xcallocator::control_alloc): Adjust spacing.
(xcallocator::data_alloc): Likewise.
(xcallocator::control_free): Likewise.
(xcallocator::data_free): Likewise.
(hash_table::hash_table): Likewise.
(hash_table::is_created): Likewise.
(hash_table::find): Likewise.
(hash_table::find_slot): Likewise.
(hash_table::remove_elt): Likewise.
(hash_table::size): Likewise.
(hash_table::elements): Likewise.
(hash_table::collisions): Likewise.
Index: gcc/hash-table.h
===
--- gcc/hash-table.h(revision 189791)
+++ gcc/hash-table.h(working copy)
@@ -43,36 +43,44 @@ struct xcallocator
};
- /* Allocate memory for COUNT control blocks. */
+/* Allocate memory for COUNT control blocks. */
template typename Type
inline Type *
xcallocator Type::control_alloc (size_t count)
- { return static_cast Type * (xcalloc (count, sizeof (Type))); }
+{
+ return static_cast Type * (xcalloc (count, sizeof (Type)));
+}
- /* Allocate memory for COUNT data blocks. */
+/* Allocate memory for COUNT data blocks. */
template typename Type
inline Type *
xcallocator Type::data_alloc (size_t count)
- { return static_cast Type * (xcalloc (count, sizeof (Type))); }
+{
+ return static_cast Type * (xcalloc (count, sizeof (Type)));
+}
- /* Free memory for control blocks. */
+/* Free memory for control blocks. */
template typename Type
inline void
xcallocator Type::control_free (Type *memory)
- { return ::free (memory); }
+{
+ return ::free (memory);
+}
- /* Free memory for data blocks. */
+/* Free memory for data blocks. */
template typename Type
inline void
xcallocator Type::data_free (Type *memory)
- { return ::free (memory); }
+{
+ return ::free (memory);
+}
/* A common function for hashing a CANDIDATE typed pointer. */
@@ -234,7 +242,7 @@ public:
};
- /* Construct the hash table. The only useful operation next is create. */
+/* Construct the hash table. The only useful operation next is create. */
template typename Element,
hashval_t (*Hash) (const Element *candidate),
@@ -243,12 +251,12 @@ template typename Element,
template typename Type class Allocator
inline
hash_table Element, Hash, Equal, Remove, Allocator::hash_table ()
- : htab (NULL)
- {
- }
+: htab (NULL)
+{
+}
- /* See if the table has been created, as opposed to constructed. */
+/* See if the table has been created, as opposed to constructed. */
template typename Element,
hashval_t (*Hash) (const Element *candidate),
@@ -257,12 +265,12 @@ template typename Element,
template typename Type class Allocator
inline bool
hash_table Element, Hash, Equal, Remove, Allocator::is_created ()
- {
-return htab != NULL;
- }
+{
+ return htab != NULL;
+}
- /* Like find_with_hash, but compute the hash value from the element. */
+/* Like find_with_hash, but compute the hash value from the element. */
template typename Element,
hashval_t (*Hash) (const Element *candidate),
@@ -271,12 +279,12 @@ template typename Element,
template typename Type class Allocator
inline Element *
hash_table Element, Hash, Equal, Remove, Allocator::find (Element
*comparable)
- {
-return find_with_hash (comparable, Hash (comparable));
- }
+{
+ return find_with_hash (comparable, Hash (comparable));
+}
- /* Like find_slot_with_hash, but compute the hash value from the element. */
+/* Like find_slot_with_hash, but compute the hash value from the element. */
template typename Element,
hashval_t (*Hash) (const Element *candidate),
@@ -286,12 +294,12 @@ template typename Element,
inline Element **
hash_table Element, Hash, Equal, Remove, Allocator
::find_slot (Element *comparable, enum insert_option insert)
- {
-return find_slot_with_hash (comparable, Hash (comparable), insert);
- }
+{
+ return find_slot_with_hash (comparable, Hash (comparable), insert);
+}
- /* Like remove_elt_with_hash, but compute the hash value from the element.
*/
+/* Like remove_elt_with_hash, but compute the hash value from the element. */
template typename Element,
hashval_t (*Hash) (const Element *candidate),
@@ -301,13 +309,12 @@ template typename Element,
inline void
hash_table Element, Hash, Equal, Remove, Allocator
::remove_elt (Element *comparable)
- {
-remove_elt_with_hash (comparable, Hash (comparable));
- }
-
+{
+ remove_elt_with_hash (comparable, Hash (comparable));
+}
- /* Return the current size of this hash table. */
+/* Return
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/7/12, Richard Guenther richard.guent...@gmail.com wrote:
On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl cr...@google.com wrote:
Convert double_int from a struct with function into a class with
operators and methods.
This patch adds the methods and operators. In general functions of
the form double_int_whatever become member functions whatever or,
when possible, operators.
Every attempt has been made to preserve the existing algorithms, even
at the expense of some optimization opportunities. Some examples:
The ext operation takes a value and returns a value. However, that
return value is usually assigned to the original variable. An
operation that modified a variable would be more efficient.
That's not always the case though and I think the interface should be
consistent with existing behavior to avoid errors creeping in during the
transition.
We should probably think about naming conventions for mutating operations,
as I expect we will want them eventually.
In some cases, an outer sign-specific function calls an inner
function with the sign as a parameter, which then decides which
implementation to do. Decisions should not be artificially
introduced, and the implementation of each case should be exposed as
a separate routine.
The existing operations are implemented in terms of the new
operations, which necessarily adds a layer between the new code and
the existing users. Once all files have migrated, this layer will
be removed.
There are several existing operations implemented in terms of even
older legacy operations. This extra layer has not been removed.
On occasion though, parameterized functions are often called
with a constant argments. To support static statement of intent,
and potentially faster code in the future, there are several new
unparameterized member functions. Some examples:
Four routines now encode both 'arithmetic or logical' and 'right or
left' shift as part of the funciton name.
Four routines now encode both 'signed or unsigned' and 'less than or
greater than' as part of the function name.
For most parts overloads that take an (unsigned) HOST_WIDE_INT argument
would be nice, as well as the ability to say dbl + 1.
Hm. There seems to be significant opinion that there should not be any
implicit conversions. I am okay with operations as above, but would like
to hear the opinions of others.
-typedef struct
+typedef struct double_int
{
+public:
+ /* Normally, we would define constructors to create instances.
+ Two things prevent us from doing so.
+ First, defining a constructor makes the class non-POD in C++03,
+ and we certainly want double_int to be a POD.
+ Second, the GCC conding conventions prefer explicit conversion,
+ and explicit conversion operators are not available until C++11.
*/
+
+ static double_int make (unsigned HOST_WIDE_INT cst);
+ static double_int make (HOST_WIDE_INT cst);
+ static double_int make (unsigned int cst);
+ static double_int make (int cst);
Did we somehow decide to not allow constructors? It's odd to convert to
C++ and end up with static member functions resembling them ...
Constructors are allowed, but PODs are often passed more efficiently.
That property seemed particularly important for double_int.
Also I believe the conversion above introduces possible migration errors.
Think of a previous
HOST_WIDE_INT a;
double_int d = uhwi_to_double_int (a);
if you write that now as
HOST_WIDE_INT a;
double_int d = double_int::make (a);
you get the effect of shwi_to_double_int. Oops.
Hm. I think the code was more likely to be wrong originally,
but I take your point.
So as an intermediate
I'd like you _not_ to introduce the make () overloads.
Okay.
Btw, if HOST_WIDE_INT == int the above won't even compile.
Is that true of any host? I am not aware of any. Anyway, it is
moot if we remove the overloads.
--
Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/7/12, Mike Stump mikest...@comcast.net wrote: On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote: Convert double_int from a struct with function into a class with operators and methods. We have a wide_int class that replaces this class. :-( Really? Where? I don't see neither definition nor use under gcc. It would have been better to just convert it. That depends on how easy it is to migrate. In the process I learned that double_int isn't actually a double int, but is a precision-controlled int. -- Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/7/12, Mike Stump mikest...@comcast.net wrote: On Aug 7, 2012, at 11:42 AM, Lawrence Crowl wrote: On 8/7/12, Mike Stump mikest...@comcast.net wrote: On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote: Convert double_int from a struct with function into a class with operators and methods. We have a wide_int class that replaces this class. :-( Really? Where? I don't see neither definition nor use under gcc. It is being developed; it is largely done, but is going through final reviews now. When we finish with those review bits, we'll send it out. It sounds like you'll beat us to the merge window, so, we'll cope with the fallout. Can I have a pointer to the interface? It would have been better to just convert it. That depends on how easy it is to migrate. In the process I learned that double_int isn't actually a double int, but is a precision-controlled int. Well, I'd call it a pair of HOST_WIDE_INTs... I think calling it a precision-controlled int is a tad optimistic. For example, if want to control it to be 2048 bits, you'd find that your hopes are misplaced. :-) Yes, well, it's clearly all relying on the host being bigger than the target. -- Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/7/12, Richard Henderson r...@redhat.com wrote:
On 08/06/2012 05:35 PM, Lawrence Crowl wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
+ return *this;
+}
+
+inline double_int
+double_int::operator -- ()
+{
+ *this - double_int_one;
+ return *this;
+}
Surely unused results there?
Typically yes, but that is the interface for that operator.
--
Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/7/12, Mike Stump mikest...@comcast.net wrote: On Aug 7, 2012, at 11:38 AM, Lawrence Crowl wrote: Hm. There seems to be significant opinion that there should not be any implicit conversions. I am okay with operations as above, but would like to hear the opinions of others. If there is an agreed upon and expected semantic, having them are useful. In the wide-int world, which replaces double_int, I think there is an agreeable semantic and I think it is useful, so, I think we should plan on having them, though, I'd be fine with punting their implementation until such time as someone needs it. If no one every needs the routine, I don't see the harm in not implementing it. At present, there are no functions equivalent to (double_int + int), so there can be no expressions that need this overload. I have no objection to adding such an overload, but if there are no objections, I would rather do it as a separate patch. -- Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/8/12, Richard Guenther richard.guent...@gmail.com wrote: On Aug 7, 2012 Lawrence Crowl cr...@google.com wrote: We should probably think about naming conventions for mutating operations, as I expect we will want them eventually. Right. In the end I would prefer explicit constructors. I don't think we're thinking about the same thing. I'm talking about member functions like mystring.append (foo). The += operator is mutating as well. Constructors do not mutate, they create. -- Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/8/12, Richard Guenther richard.guent...@gmail.com wrote: On Aug 7, 2012 Lawrence Crowl cr...@google.com wrote: On 8/7/12, Richard Guenther richard.guent...@gmail.com wrote: For most parts overloads that take an (unsigned) HOST_WIDE_INT argument would be nice, as well as the ability to say dbl + 1. Hm. There seems to be significant opinion that there should not be any implicit conversions. I am okay with operations as above, but would like to hear the opinions of others. Well, I'd simply add double_int operator+(HOST_WIDE_INT); double_int operator+(unsigned HOST_WIDE_INT); and be done with it ;) Yes, a tad bit more inconvenient on the implementation side compared to a non-explicit constructor from HOST_WIDE_INT or a conversion operator ... but adhering to the rule that we do _not_ want such automatic conversions (well, yet, for the start). Ah, we have a different definition of implicit conversion. In my mind, the above overloads do an implicit conversion. The issue is that the calling code does not make the conversion obvious. Note the difference in a = b + i; a = b + double_int(i); Using overloads as above is generally safer than using an implicit converting constructor or an implicit conversion operator. If folks are really only objecting to the implementation technique, we have a somewhat different outcome. -- Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/8/12, Richard Guenther richard.guent...@gmail.com wrote:
On Aug 8, 2012 Marc Glisse marc.gli...@inria.fr wrote:
On Wed, 8 Aug 2012, Richard Guenther wrote:
+ static double_int make (unsigned HOST_WIDE_INT cst);
+ static double_int make (HOST_WIDE_INT cst);
+ static double_int make (unsigned int cst);
+ static double_int make (int cst);
[...]
Btw, if HOST_WIDE_INT == int the above won't even compile.
Is that true of any host? I am not aware of any. Anyway,
it is moot if we remove the overloads.
Right. I'd simply rely on int / unsigned int promotion to
HOST_WIDE_INT or unsigned HOST_WIDE_INT and only provide
overloads for HOST_WIDE_INT kinds anyway.
Sadly, that doesn't work with the current C++ rules (there is
a proposal to change that for C++1y):
void f(long);
void f(unsigned long);
void g(int x){f(x);}
e.cc: In function ‘void g(int)’:
e.cc:3:18: error: call of overloaded ‘f(int)’ is ambiguous
e.cc:3:18: note: candidates are:
e.cc:1:6: note: void f(long int)
e.cc:2:6: note: void f(long unsigned int)
Ick ... I forgot that integer promotions do not apply for int
- long. So even f(1) would be ambiguous, right? So I suppose
we have to bite the bullet and add overloads for all (unsigned)
integer types from int to HOST_WIDE_INT (if HOST_WIDE_INT is
long long then we have to add overloads for int, unsigned int,
long and unsigned long).
Or use template magic to force promotion to HOST_WIDE_INT for
integer types smaller than HOST_WIDE_INT...
I did not get to the structure I had accidentally. However, with
the prior suggestion to preserve exact semantics to existing calls,
it is all moot because we cannot have overloading.
We can test whether the code is making double_ints with cross-sign
ints by adding undefined overloads. I think we should do that and
make all such crossings explicit. However, I want to wait for a
separate patch.
--
Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/8/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Aug 8, 2012 Miles Bader mi...@gnu.org wrote:
Richard Guenther richard.guent...@gmail.com writes:
Constructors are allowed, but PODs are often passed
more efficiently. That property seemed particularly
important for double_int.
Show us the difference in timing. Show us the generated
code. I can't imagine that it could ever matter.
I'm also curious about that statement... PODs don't really
seem to offer much advantage with modern compilers, except
in a few very specific cases (of which this doesn't seem
to be one), e.g. in unions.
They make a difference for the by-value passing ABI.
double-ints can be passed in two registers on most platforms.
Sure, but that doesn't seem to depend on PODness -- non-PODs
can be passed in two registers as well, AFAICS...
E.g., in the following:
typedef long valtype;
struct X { valtype x, y; };
struct Y { valtype x, y;
Y (valtype a, valtype b) : x (a), y (b) { }
};
extern void fx (X x);
void test_x () {X x = { 1, 2 }; fx (x); }
extern void fy (Y y);
void test_y () {Y y (1, 2); fy (y); }
test_x and test_y use exactly the same calling sequence (and
contain exactly the same assembly code)... [on x86-64]
It is not PODness in the standard sense that matters. It is
podness from the ABI perspecitve: Y does not have a user-defined
copy-constructor nor a desctructor; it is not a polymorphic type,
so it is OK to pass in registers just like X.
Well, more specifically, the Itanium ABI says if the copy constructor
and the destructor are trivial, one passes the class in registers.
Other ABIs can make other choices. Older ABIs often pass all
structs and classes indirectly.
The adding in inline versions of various special members into
the following code results in the following worst-case changes.
x86 x86_64
instructions: 48 - 69 = 144% 38 - 53 = 139%
stack operations:6 - 10 = 167% 2 - 6 = 300%
memory operations: 28 - 38 = 136% 22 - 28 = 127%
These numbers are not huge, particularly in context, but worth
considering.
In any event, the point is moot. To implement the exact semantics of
current expressions using shwi_to_double_int and uhwi_to_double_int,
we cannot use constructors anyway. So, there remains no immediate
reason to use any constructors.
struct type
{
#ifdef DEFAULT
type ();
#endif
#ifdef INITIAL
type (int);
#endif
#ifdef COPYCON
type (const type from)
: field1 (from.field1),
field2 (from.field2)
{
}
#endif
#ifdef DESTRUCT
~type ()
{
field1 = 0;
field2 = 0;
}
#endif
type method (type);
long int field1;
long int field2;
};
extern type global;
void callee (type arg);
void function_caller ()
{
type local (global);
callee (local);
callee (global);
}
void method_caller (type arg1, type arg2)
{
type var1 (global);
type var2 = arg2;
arg1.method (arg2);
var1.method (var2);
}
--
Lawrence Crowl
Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)
On 8/8/12, Miles Bader mi...@gnu.org wrote: Mike Stump mikest...@comcast.net writes: Constructors are allowed, but PODs are often passed more efficiently. That property seemed particularly important for double_int. Show us the difference in timing. Show us the generated code. I can't imagine that it could ever matter. I'm also curious about that statement... PODs don't really seem to offer much advantage with modern compilers, except in a few very specific cases (of which this doesn't seem to be one), e.g. in unions. Which brings up a critical point that double_int is used in trees, which are in a union, and so double_int must be a POD until trees are no longer unions. -- Lawrence Crowl
[cxx-conversion] Avoid overloaded double_int 'constructor'. (issue6441127)
Convert overloaded double_int::make to non-overloaded from_signed and
from_unsigned. This change is intended to preserve the exact semantics of
the existing expressions using shwi_to_double_int and uhwi_to_double_int.
Tested on x86_64.
Index: gcc/ChangeLog
2012-08-09 Lawrence Crowl cr...@google.com
* double-int.h (double_int::make): Remove.
(double_int::from_signed): New.
(double_int::from_unsigned): New.
(shwi_to_double_int): Use double_int::from_signed instead of
double_int::make.
(double_int_minus_one): Likewise.
(double_int_zero): Likewise.
(double_int_one): Likewise.
(double_int_two): Likewise.
(double_int_ten): Likewise.
(uhwi_to_double_int): Use double_int::from_unsigned instead of
double_int::make.
Index: gcc/double-int.h
===
--- gcc/double-int.h(revision 190239)
+++ gcc/double-int.h(working copy)
@@ -60,10 +60,8 @@ public:
Second, the GCC conding conventions prefer explicit conversion,
and explicit conversion operators are not available until C++11. */
- static double_int make (unsigned HOST_WIDE_INT cst);
- static double_int make (HOST_WIDE_INT cst);
- static double_int make (unsigned int cst);
- static double_int make (int cst);
+ static double_int from_unsigned (unsigned HOST_WIDE_INT cst);
+ static double_int from_signed (HOST_WIDE_INT cst);
/* No copy assignment operator or destructor to keep the type a POD. */
@@ -188,7 +186,7 @@ public:
HOST_WIDE_INT are filled with the sign bit. */
inline
-double_int double_int::make (HOST_WIDE_INT cst)
+double_int double_int::from_signed (HOST_WIDE_INT cst)
{
double_int r;
r.low = (unsigned HOST_WIDE_INT) cst;
@@ -196,17 +194,11 @@ double_int double_int::make (HOST_WIDE_I
return r;
}
-inline
-double_int double_int::make (int cst)
-{
- return double_int::make (static_cast HOST_WIDE_INT (cst));
-}
-
/* FIXME(crowl): Remove after converting callers. */
static inline double_int
shwi_to_double_int (HOST_WIDE_INT cst)
{
- return double_int::make (cst);
+ return double_int::from_signed (cst);
}
/* Some useful constants. */
@@ -214,17 +206,17 @@ shwi_to_double_int (HOST_WIDE_INT cst)
The problem is that a named constant would not be as optimizable,
while the functional syntax is more verbose. */
-#define double_int_minus_one (double_int::make (-1))
-#define double_int_zero (double_int::make (0))
-#define double_int_one (double_int::make (1))
-#define double_int_two (double_int::make (2))
-#define double_int_ten (double_int::make (10))
+#define double_int_minus_one (double_int::from_signed (-1))
+#define double_int_zero (double_int::from_signed (0))
+#define double_int_one (double_int::from_signed (1))
+#define double_int_two (double_int::from_signed (2))
+#define double_int_ten (double_int::from_signed (10))
/* Constructs double_int from unsigned integer CST. The bits over the
precision of HOST_WIDE_INT are filled with zeros. */
inline
-double_int double_int::make (unsigned HOST_WIDE_INT cst)
+double_int double_int::from_unsigned (unsigned HOST_WIDE_INT cst)
{
double_int r;
r.low = cst;
@@ -232,17 +224,11 @@ double_int double_int::make (unsigned HO
return r;
}
-inline
-double_int double_int::make (unsigned int cst)
-{
- return double_int::make (static_cast unsigned HOST_WIDE_INT (cst));
-}
-
/* FIXME(crowl): Remove after converting callers. */
static inline double_int
uhwi_to_double_int (unsigned HOST_WIDE_INT cst)
{
- return double_int::make (cst);
+ return double_int::from_unsigned (cst);
}
inline double_int
--
This patch is available for review at http://codereview.appspot.com/6441127
Re: [cxx-conversion] Support garbage-collected C++ templates
On 8/10/12, Diego Novillo dnovi...@google.com wrote: On 12-08-10 16:14 , Richard Henderson wrote: On 2012-08-10 08:06, Diego Novillo wrote: The end point should be that the only thing we really need to tell gengtype about are the variable roots. Everything else would rely on user-provided markings. I suppose we could still keep the automatic option for really simple stuff, though. Yes please. Markup may be awkward, but I think can be less awkward than generating those three cookie-cutter functions. Yeah. Another thing I suppose we want to keep is the whole 'prev' and 'next' markings. These generate harness that would be cumbersome to implement from user code. Diego and I talked about this issue. What follows is sort of a brain dump. Corrections welcome. There are several principles we can apply to the work. Data types that change frequently should be handled automatically. Otherwise, the purpose of precise garbage collection is somewhat defeated. If we get to that situation, we should simply figure out how to convert the collector to something else. GTY markings that change frequently, or are hard to understand and get right, are a burden as well. The complexity of gengtype's implementation can be substantially reduced if it does not try to solve all problems. We are compiler engineers, and we can do complex manual work when appropriate. Not everything needs to be automatic. Applying those principles, we can reach some conclusions about approach. We can rely on GTY((user)) for template class types. By and large these types implement general data structures, like hash tables. The won't change much once built, and so maintenance of the manual marking routines will not be a large burden. Taking this approach means that gengtype will not have to understand templates. We really do not want gengtype trying to understand templates. Diego has already implemented this part. Some types generic via void* and use param_is/use_param. As we migrate to templates, much of these uses can go away. Perhaps, eventually, we might be able to remove that complexity from gengtype. As this possibility is closer to wishful thinking, it has really low priority. We cannot rely on GTY((user)) for class hierarchies that hold program structures. For example, cgraph and tree nodes are too volatile to rely on manually synchronizing the marker function with the data fields. So, we need to get gengtype to work with class hierarchies. We probably do not need to handle multiple inheritance. First, existing structures do not use it. Second, it probably would not be used much anyway. Polymorphic class hierarchies may require the user to add a couple of method declarations for gengtype. Gengtype would then provide the method definitions. We do not have any polymorphic classes yet, so this issue has a low priority. We will likely borrow the descriminator/tag structure of unions for the marking of non-polymorphic class hierarchies. Gengtype will split the marking of fields and the identification of the dynamic type into separate functions. We have several non-polymorphic class hierarchies now, though represented with unions and embedding, so this issue has a high priority. Some current classes with nested unions, e.g. tree_type_symtab within tree_type_common within tree_node, might be convertable to a class hierarchy that eliminates all unions. If so, they will likely need more than one descriminator. Given that such structures can persist with nested unions for some time, this issue has low priority. Given all that, it seems that the highest priority for gengtype modifications is direct support for non-polymorphic single-inheritance class hierarchies that are discriminated by tags. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Aug 13, 2012 Marc Glisse marc.gli...@inria.fr wrote:
On Mon, 13 Aug 2012, Jakub Jelinek wrote:
On Sun, Aug 12, 2012 at 11:30:59PM +0200, Marc Glisse wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Do you mean that *this + double_int_one; alone also works,
just is confusing? That would mean operator+ has side-effects,
right?
It works in that it compiles. It is confusing because the
addition is dead code and thus operator++ is a nop. Sorry for
my confusing euphemism, I should have called it a bug. operator+
has no side-effects AFAICS.
yes, it is just as confusing and a bug as
2.3 + 1;
is in plain C.
Yes, it is a bug. It's a bit disturbing that it wasn't caught
in bootstrap.
Note that there are many obvious places where this operator can be used:
varasm.c: i = double_int_add (i, double_int_one);
tree-vrp.c: prod2h = double_int_add (prod2h, double_int_one);
tree-ssa-loop-niter.c:bound = double_int_add (bound, double_int_one);
tree-ssa-loop-niter.c: *nit = double_int_add (*nit, double_int_one);
tree-ssa-loop-ivopts.c:max_niter = double_int_add (max_niter,
double_int_one);
gimple-fold.c:index = double_int_add (index, double_int_one);
etc.
As a side note, I don't usually like making operator+ a
member function. It doesn't matter when there is no implicit
conversion, but if we ever add them, it will make addition
non-symmetric.
As not everybody is familiar with C++ litotes, let me expand
on this. I believe you are not opposing overloading operator+ on
double_int. You are objecting to its implementation being defined
as a member function. That is you would be perfectly fine with
operator+ defined as a free function, e.g. not a member function.
In the absence of symmetric overloading, the code is slightly
cleaner with operators as member functions. It is easy to change
should we need symmetric overloads because, for the most part,
the change would have no effect on client code.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/12/12, Marc Glisse marc.gli...@inria.fr wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements the double_int rewrite.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
I am taking it as a chance to ask a couple questions about the coding
conventions.
2012-08-12 Lawrence Crowl cr...@google.com
* hash-table.h
(typedef double_int): Change to struct (POD).
(double_int::make): New overloads for int to double-int conversion.
Isn't that double_int::from_* now?
Yes.
+typedef struct double_int
{
[...]
} double_int;
Does the coding convention say something about this verbosity?
No. It helps to have it in code that is compiled by both C and C++.
In this case, it will only be compiled by C++ and the verbosity
is unnecessary. I left the verbosity as it was to help keep the
diff synchronized. I certainly don't object to a cleanup pass for
this kind of stuff.
+ HOST_WIDE_INT to_signed () const;
+ unsigned HOST_WIDE_INT to_unsigned () const;
+
+ /* Conversion query functions. */
+
+ bool fits_unsigned() const;
+ bool fits_signed() const;
Space before the parentheses or not?
Space. Sorry, gcc is the only coding convention I've used that
requires the space. My fingers sometimes forget.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Richard Guenther richard.guent...@gmail.com wrote: Increment/decrement operations did not exist, please do not add them at this point. Note that I have also added +=, -= and *= operations. Having them has three advantages. First, it matches expectations on what numeric types allow. Second, it results in more concise code. Third, it results in potentially faster code. I think we should be able to use those operators. When I run through changing call sites, I really want to change the sites to the final form, not do two passes. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Richard Henderson r...@redhat.com wrote: On 08/13/2012 01:22 PM, Lawrence Crowl wrote: yes, it is just as confusing and a bug as 2.3 + 1; is in plain C. Yes, it is a bug. It's a bit disturbing that it wasn't caught in bootstrap. You'll recall that I pointed it out last time around as well. My memory must be losing bits. -- Lawrence Crowl
Re: [patch] timevar TLC
On 8/14/12, Steven Bosscher stevenb@gmail.com wrote:
On Aug 14, 2012 Diego Novillo dnovi...@google.com wrote:
On 12-08-14 14:26 , Steven Bosscher wrote:
Many unused timevars, many timevars that measure completely
different passes, passes with the wrong timevar, etc.
Time for a bit of maintenance / janitorial.
Bootstrappedtested on powerpc64-unknown-linux-gnu. OK
for trunk?
* timevar.def (TV_VARPOOL, TV_WHOPR_WPA_LTRANS_EXEC, TV_LIFE,
TV_LIFE_UPDATE, TV_DF_UREC, TV_INLINE_HEURISTICS,
TV_TREE_LINEAR_TRANSFORM, TV_TREE_LOOP_INIT, TV_TREE_LOOP_FINI,
TV_VPT, TV_LOCAL_ALLOC, TV_GLOBAL_ALLOC, TV_SEQABSTR): Remove.
(TV_IPA_INLINING, TV_FLATTEN_INLINING, TV_EARLY_INLINING,
TV_INLINE_PARAMETERS, TV_LOOP_INIT, TV_LOOP_FINI): New.
* timevar.c (timevar_print): Make printing width of timevar
names
more flexible, but enforce maximum length.
* ipa-inline.c (pass_early_inline): Use TV_EARLY_INLINING.
(pass_ipa_inline): Use TV_IPA_INLINING.
* ipa-inline-analysis.c (pass_inline_parameters): Use
TV_INLINE_HEURISTICS.
* tree-ssa-loop.c (pass_tree_loop_init): No timevar for wrapper
pass.
(pass_tree_loop_done): Likewise.
* final.c (pass_shorten_branches): Use TV_SHORTEN_BRANCH.
* loop-init.c (loop_optimizer_init): Push/pop TV_LOOP_INIT.
(loop_optimizer_finalize): Push/pop TV_LOOP_FINI.
Looks fine, except:
@@ -505,6 +507,16 @@ timevar_print (FILE *fp)
TIMEVAR. */
start_time = now;
+#ifdef ENABLE_CHECKING
+ /* Pester those who add timevars with too long names. */
+ for (id = 0; id (unsigned int) TIMEVAR_LAST; ++id)
+{
+ struct timevar_def *tv = timevars[(timevar_id_t) id];
+ if ((timevar_id_t) id != TV_TOTAL tv-used)
+ gcc_assert (strlen (tv-name) = name_width);
+}
+#endif
I'm not liking this too much. I would rather do truncation
or wrapping. Not ICEing. And we'd do this all the time,
not just with checking enabled.
Wrapping would be bad, it'd break scripts that parse the
-ftime-report output. Truncation is a bit silly, too: If the name
is always truncated, why have the long name in the first place?
I chose for this gcc_assert solution to make absolutely sure
nobody can add a timevar with an overlong name.
I suppose this works with -ftime-report right? (I'm thinking
about the new code that emits phase-level timers).
Yes, I've been using -ftime-report a lot for PR54146, and the
output format was itching...
You can check the error statically. Something like
% cat limitstring.c
#define LIMIT 32
struct def {
int x;
char name[LIMIT+1];
};
struct def var[] = {
{ 3, hello },
{ 4, name is much too too long for a reasonable name },
};
% gcc -c limitstring.c -Werror
cc1: warnings being treated as errors
limitstring.c:10: error: initializer-string for array of chars is too long
limitstring.c:10: error: (near initialization for 'timevars[1].name')
But of course the variable definition would look more like
#define DEFTIMEVAR(identifier__, name__) \
{ , name__, ... },
struct def var[] = {
#include timevar.def
};
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Guenther rguent...@suse.de wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements a new C++ hash table.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Now as we see the result I'd have prefered a more C++-way instead
of making the conversion simple ...
Like
template typename Element
class hash_table
{
...
};
template typename Element
class pointer_hash
{
hashval_t hash ();
int equal (const Element *);
~Element ();
Element e;
};
and
/* Hash table with all same_succ entries. */
static hash_table pointer_hash struct same_succ_def same_succ_htab;
The existing way is simply too ugly ... so, why did you not invent
a nice C++ way? (please consider reverting the hashtable patch)
We are trying to balance several factors. Sometimes we're going
to pick multiple steps rather than a single step. In some cases,
as here, the intent was to make the client code changes minimal and
unsurprising while still getting the type safety and efficiency.
Other times, it may be a minor technical issue. In particular,
I prefer to avoid steps that might cause very poor matching of
lines in the diff. Others may choose differently. Together we
will learn where the tradeoffs lie.
More in a later response.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Guenther rguent...@suse.de wrote: On Wed, 15 Aug 2012, Michael Matz wrote: On Wed, 15 Aug 2012, Richard Guenther wrote: Like the following, only the coverage.c use is converted. I've never seen template function arguments anywhere and having to repeat them all over the place is really really ugly (yes, even if only in the implementation). This goes with static member functions and not wrapping any data. It goes away with the requirement of having externally visible global functions for the hash/compare functions as well (which was ugly anyway). Well, it looks nicer than what's there currently. As the element functions now are scoped and normal member functions, they should be named with lower case characters of course. I do like that the hash table would then only have one real template argument; the Allocator, well, no better idea comes to my mind. Yeah. Updated patch below, all users converted. I probably missed to revert some of the globalizations of hash/compare fns. Your conversion is a better abstraction, and something I'd wanted to get to eventually, so I support your conversion. BTW, the conding conventions say to put member function definitions out of line. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I do not much like _t names either. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/16/12, Richard Guenther rguent...@suse.de wrote:
On Wed, 15 Aug 2012, Lawrence Crowl wrote:
On 8/15/12, Richard Henderson r...@redhat.com wrote:
On 2012-08-15 07:29, Richard Guenther wrote:
+ typedef typename Element::Element_t Element_t;
Can we use something less ugly than Element_t?
Such as
typedef typename Element::T T;
? Given that this name is scoped anyway...
I do not much like _t names either.
The following is what I'm testing now, it also integrates the
hashtable support functions and typedef within the existing local
data types which is IMHO cleaner. (it also shows we can do with
a janitorial cleanup replacing typedef struct foo_d {} foo; with
struct foo {}; and the likes)
Yes.
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu, ok?
Looks good to me.
I would have prefered the Element-T rename in a separate patch
so that it is easier to see the core difference.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 8/20/12, H.J. Lu hjl.to...@gmail.com wrote: The C++ merge caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54332 GCC memory usage is more than doubled from = 3GB to = 10GB. Is this a known issue? The two memory stat reports show no differences. Are you sure you didn't splice in the wrong report? -- Lawrence Crowl
Re: Change double_int calls to new interface.
On 9/5/12, Richard Guenther rguent...@suse.de wrote:
On Tue, 4 Sep 2012, Lawrence Crowl wrote:
Modify gcc/*.[hc] double_int call sites to use the new interface.
This change entailed adding a few new methods to double_int.
Other changes will happen in separate patches. Once all uses of
the old interface are gone, they will be removed.
The change results in a 0.163% time improvement with a 70%
confidence.
Tested on x86_64.
Index: gcc/ChangeLog
- double_int_lshift
- (double_int_one,
-TREE_INT_CST_LOW (vr1.min),
-TYPE_PRECISION (expr_type),
-false));
+ double_int_one
+ .llshift (TREE_INT_CST_LOW
(vr1.min),
+ TYPE_PRECISION
(expr_type)));
Ick - is that what our coding-conventions say? I mean the
.llshift on the next line.
Our conventions say nothing about that, but method calls seem
somewhat analogoust to binary operators, and hence this formatting
was probably the least objectional.
Otherwise ok.
As in you want me to do something else?
The tmin.cmp (tmax, uns) 0 kind of things look odd - definitely
methods like tmin.gt (tmax, uns) would be nice to have. Or even
better, get rid of the 'uns' parameters and provide a
struct double_int_with_signedness {
double_int val;
bool uns;
};
struct double_uint : double_int_with_signedness {
double_uint (double_int);
};
...
and comparison operators which take double_uint/sint.
It would, I think, be better to have separate signed and unsigned
types. That change was significantly structural, and I don't know
where the wide_int work sits in relation to that choice.
You didn't remove any of the old interfaces, so I think we are
going to bitrot quickly again.
I couldn't remove the old interface yet because I haven't updated
all the code yet.
--
Lawrence Crowl
Re: Change double_int calls to new interface.
On 9/5/12, Marc Glisse marc.gli...@inria.fr wrote:
On Wed, 5 Sep 2012, Lawrence Crowl wrote:
On 9/5/12, Richard Guenther rguent...@suse.de wrote:
The tmin.cmp (tmax, uns) 0 kind of things look odd - definitely
methods like tmin.gt (tmax, uns) would be nice to have. Or even
better, get rid of the 'uns' parameters and provide a
struct double_int_with_signedness {
double_int val;
bool uns;
};
struct double_uint : double_int_with_signedness {
double_uint (double_int);
};
...
and comparison operators which take double_uint/sint.
It would, I think, be better to have separate signed and unsigned
types. That change was significantly structural, and I don't know
where the wide_int work sits in relation to that choice.
Note that in tree-vrp.c, if I remember correctly, I used both signed and
unsigned operations on the same object (emulating arbitrary precision is
a pain).
Presumably the wide_int work will address that issue.
--
Lawrence Crowl
Re: Change double_int calls to new interface.
On 9/11/12, Andreas Schwab sch...@linux-m68k.org wrote: Mark Kettenis mark.kette...@xs4all.nl writes: In file included from ../../../src/gcc/gcc/mcf.c:47:0: ../../../src/gcc/gcc/mcf.c: In function 'void dump_fixup_edge(FILE*, fixup_graph_type*, fixup_edge_p)': ../../../src/gcc/gcc/system.h:288:78: error: integer overflow in expression [-Werror=overflow] This is PR54528. The expression itself looks correct. I have not been able to duplicate the problem on x86. I am now waiting on access to the compile farm for access to a hppa system. Does anyone have more specific information on the condition that generates the error? -- Lawrence Crowl
Re: Backtrace library [1/3]
On 9/11/12, Ian Lance Taylor i...@google.com wrote: This patch is the interface to and configury of libbacktrace. I've separated these out as the parts of libbacktrace that require the most review. The interface to libbacktrace is in the file backtrace.h. This is what callers will use. The file backtrace-supported.h is also available so that programs can see whether calling the backtrace library will work at all. The interface relies on global data in the library. Wouldn't it be better to expose the state as an additional parameter to enable concurrent access by different threads? That parameter could then be modeled as 'this' parameter, addressing Gaby's suggesting. -- Lawrence Crowl
Re: Backtrace library [1/3]
On 9/12/12, Ian Lance Taylor i...@google.com wrote: On Sep 11, 2012 Lawrence Crowl cr...@googlers.com wrote: On 9/11/12, Ian Lance Taylor i...@google.com wrote: This patch is the interface to and configury of libbacktrace. I've separated these out as the parts of libbacktrace that require the most review. The interface to libbacktrace is in the file backtrace.h. This is what callers will use. The file backtrace-supported.h is also available so that programs can see whether calling the backtrace library will work at all. The interface relies on global data in the library. Wouldn't it be better to expose the state as an additional parameter to enable concurrent access by different threads? That parameter could then be modeled as 'this' parameter, addressing Gaby's suggesting. I went ahead and added a state parameter to the interface. I've attached the updated patch. How about typing it as a pointer to an incomplete struct? extern void *backtrace_create_state (... becomes, e.g., struct backtrace_state; extern backtrace_state *backtrace_create_state (... -- Lawrence Crowl
Re: Change double_int calls to new interface.
On 9/12/12, Mark Kettenis mark.kette...@xs4all.nl wrote: Date: Tue, 11 Sep 2012 17:03:39 -0700 From: Ian Lance Taylor i...@google.com On Tue, Sep 11, 2012 at 3:12 PM, Lawrence Crowl cr...@googlers.com wrote: On 9/11/12, Andreas Schwab sch...@linux-m68k.org wrote: Mark Kettenis mark.kette...@xs4all.nl writes: In file included from ../../../src/gcc/gcc/mcf.c:47:0: ../../../src/gcc/gcc/mcf.c: In function 'void dump_fixup_edge(FILE*, fixup_graph_type*, fixup_edge_p)': ../../../src/gcc/gcc/system.h:288:78: error: integer overflow in expression [-Werror=overflow] This is PR54528. The expression itself looks correct. I have not been able to duplicate the problem on x86. I am now waiting on access to the compile farm for access to a hppa system. Does anyone have more specific information on the condition that generates the error? I haven't tried, but I bet you can get it to happen if you build a 32-bit compiler--that is, build a compiler using -m32 so that the compiler itself runs 32 bit code. I don't think that helps since I don't see the problem on OpenBSD/i386 (i386-unknown-openbsd5.2) whith a strictly 32-bit compiler. As I wrote earlier, I suspect the key factor is HOST_WIDE_INT being 32-bit, which means 32-bit architectures that don't sey need_64bit_hwint in config.gcc. The fact that m68k is affected as well strengthens my suspicion. So I expect arm to show the problem as well. But people probably haven't noticed since they cross-compile. Anyway the diff below seems to fix the issue. Guess the replacement doesn't work! Should be a big enough clue for Lawrence to come up with a proper fix. Index: fold-const.c === --- fold-const.c (revision 191150) +++ fold-const.c (working copy) @@ -982,13 +982,15 @@ break; case MINUS_EXPR: -/* FIXME(crowl) Remove this code if the replacment works. + /* FIXME(crowl) Remove this code if the replacment works. */ +#if 1 neg_double (op2.low, op2.high, res.low, res.high); add_double (op1.low, op1.high, res.low, res.high, res.low, res.high); overflow = OVERFLOW_SUM_SIGN (res.high, op2.high, op1.high); -*/ +#else res = op1.add_with_sign (-op2, false, overflow); +#endif break; case MULT_EXPR: My suspicions were on the shift operation. I will update the last patch I sent out with a fix. I will probably need help testing. -- Lawrence Crowl
Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)
On 7/15/11, Gabriel Charette gch...@google.com wrote:
This patch adds an expected checksum for the tests expecting an asm diff.
This way if we were expecting an asm diff, still get one, but a different
one, we know (before this patch we would ignore this, generating an XFAIL as
usual, as the status of having an asm diff itself hadn't changed).
I had to change from using the TCL grep to the bash grep (through an exec
call) as I now need the actual output of the grep call, not only the return
value (it also turns out the return value for TCL grep and bash grep are
different; hence the change in the if statements on the adiff variable)
Gab
2011-07-15 Gabriel Charette gch...@google.com
* g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
* g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
* g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
* lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.
diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
index c2fceec..6887b11 100644
--- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
+++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
@@ -1,2 +1,2 @@
-// pph asm xdiff
+// pph asm xdiff 52758
#include c0builtin-integral.h
diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
index b127f98..3321870 100644
--- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
@@ -1,5 +1,5 @@
// { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 36206
#include c0eabi1.h
diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
index 4247f49..2f0715f 100644
--- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
@@ -1,5 +1,5 @@
// { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 15662
#include p4eabi1.h
diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
index b706f27..1d7deed 100644
--- a/gcc/testsuite/lib/dg-pph.exp
+++ b/gcc/testsuite/lib/dg-pph.exp
@@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix }
{
verbose -log
# Compare the two assembly files. They should be identical.
-set adiff [diff $bname.s-pph $bname.s+pph]
+set adiff [catch {exec diff $bname.s-pph $bname.s+pph} diff_result]
# The sources mark when they expect the comparison to differ.
-set xdiff [llength [grep $test pph asm xdiff]]
+set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
+set xdiff [llength $xdiff_entry]
if { $adiff == 0 } {
- fail $nshort $options comparison failure
-} elseif { $adiff == 1 } {
if { $xdiff } {
xpass $nshort $options (assembly comparison)
} else {
@@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix }
{
}
file_on_host delete $bname.s-pph
file_on_host delete $bname.s+pph
-} else {
+} elseif { $adiff == 1 } {
+verbose -log Diff obtained:\n$diff_result
if { $xdiff } {
- xfail $nshort $options (assembly comparison)
+ set expectedSum [exec tr -d \} [exec cut -f 4 -d\
$xdiff_entry]]
+ set actualSum [exec cut -f 1 -d\ [exec sum $diff_result]]
+ if { $expectedSum == $actualSum } {
+ xfail $nshort $options (assembly comparison)
+ } else {
+ fail $nshort $options (assembly comparison, sums differ:
expected
$expectedSum, actual $actualSum)
+ }
} else {
fail $nshort $options (assembly comparison)
}
+} else {
+ fail $nshort $options comparison failure
}
}
--
This patch is available for review at http://codereview.appspot.com/4744043
Needs shortening of message. Otherwise, LGTM.
--
Lawrence Crowl
Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)
On 7/15/11, Lawrence Crowl cr...@google.com wrote:
On 7/15/11, Gabriel Charette gch...@google.com wrote:
This patch adds an expected checksum for the tests expecting an asm diff.
This way if we were expecting an asm diff, still get one, but a different
one, we know (before this patch we would ignore this, generating an XFAIL
as
usual, as the status of having an asm diff itself hadn't changed).
I had to change from using the TCL grep to the bash grep (through an exec
call) as I now need the actual output of the grep call, not only the
return
value (it also turns out the return value for TCL grep and bash grep are
different; hence the change in the if statements on the adiff variable)
Gab
2011-07-15 Gabriel Charette gch...@google.com
* g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
* g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
* g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
* lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.
diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
index c2fceec..6887b11 100644
--- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
+++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
@@ -1,2 +1,2 @@
-// pph asm xdiff
+// pph asm xdiff 52758
#include c0builtin-integral.h
diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
index b127f98..3321870 100644
--- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
@@ -1,5 +1,5 @@
// { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 36206
#include c0eabi1.h
diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
index 4247f49..2f0715f 100644
--- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
@@ -1,5 +1,5 @@
// { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 15662
#include p4eabi1.h
diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
index b706f27..1d7deed 100644
--- a/gcc/testsuite/lib/dg-pph.exp
+++ b/gcc/testsuite/lib/dg-pph.exp
@@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix
}
{
verbose -log
# Compare the two assembly files. They should be identical.
-set adiff [diff $bname.s-pph $bname.s+pph]
+set adiff [catch {exec diff $bname.s-pph $bname.s+pph}
diff_result]
# The sources mark when they expect the comparison to differ.
-set xdiff [llength [grep $test pph asm xdiff]]
+set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
+set xdiff [llength $xdiff_entry]
if { $adiff == 0 } {
-fail $nshort $options comparison failure
-} elseif { $adiff == 1 } {
if { $xdiff } {
xpass $nshort $options (assembly comparison)
} else {
@@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix
}
{
}
file_on_host delete $bname.s-pph
file_on_host delete $bname.s+pph
-} else {
+} elseif { $adiff == 1 } {
+verbose -log Diff obtained:\n$diff_result
if { $xdiff } {
-xfail $nshort $options (assembly comparison)
+set expectedSum [exec tr -d \} [exec cut -f 4 -d\
$xdiff_entry]]
+set actualSum [exec cut -f 1 -d\ [exec sum $diff_result]]
+if { $expectedSum == $actualSum } {
+xfail $nshort $options (assembly comparison)
+} else {
+fail $nshort $options (assembly comparison, sums differ:
expected
$expectedSum, actual $actualSum)
+}
} else {
fail $nshort $options (assembly comparison)
}
+} else {
+fail $nshort $options comparison failure
}
}
--
This patch is available for review at
http://codereview.appspot.com/4744043
Needs shortening of message. Otherwise, LGTM.
We have crossed the streams. LGTM.
--
Lawrence Crowl
