* [PATCH] bitmap: Introduce bitmap_head_pod
@ 2023-09-28 9:12 Jakub Jelinek
2023-09-28 9:29 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2023-09-28 9:12 UTC (permalink / raw)
To: Richard Biener, Richard Sandiford; +Cc: gcc-patches
Hi!
The following patch splits the bitmap_head class into a POD
struct bitmap_head_pod and bitmap_head derived from it with non-trivial
default constexpr constructor. Most code should keep using bitmap_head
as before, bitmap_head_pod is there just for the cases where we want to
embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
and in a loop bitmap_initialize it afterwards (to avoid having to
{quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
The patch is larger than I hoped, because e.g. some code just used bitmap
and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-09-28 Jakub Jelinek <jakub@redhat.com>
* coretypes.h (struct bitmap_head_pod): New forward declaration.
(bitmap, const_bitmap): Replace bitmap_head with bitmap_head_pod
in typedefs.
* bitmap.h (struct bitmap_obstack): Change heads element type to
bitmap_head_pod *.
(struct bitmap_head_pod): New type.
(class bitmap_head): Changed into derived class from bitmap_head_pod
with a non-trivial constexpr default ctor.
* bitmap.cc (bitmap_head::crashme): Remove.
(bitmap_head_pod::crashme): New static data member.
(debug): Change bitmap_head to bitmap_head_pod.
(bitmap_head::dump): Renamed to ...
(bitmap_head_pod::dump): ... this.
* tree-ssa-live.h (compute_live_vars, live_vars_at_stmt,
destroy_live_vars): Change vec<bitmap_head> to vec<bitmap_head_pod>.
* tree-ssa-live.cc (struct compute_live_vars_data): Change active
member type to vec<bitmap_head_pod>.
(compute_live_vars, live_vars_at_stmt, destroy_live_vars): Change
vec<bitmap_head> to vec<bitmap_head_pod>.
* sel-sched-ir.h (forced_ebb_heads): Change type to bitmap.
* gimple-range-path.cc (path_range_query::path_range_query,
path_range_query::reset_path, path_range_query::compute_ranges):
Change const bitmap_head * in argument types to const_bitmap.
* gimple-range-path.h (path_range_query::path_range_query,
path_range_query::reset_path, path_range_query::compute_ranges):
Likewise.
(path_range_query::compute_exit_dependencies): Change bitmap_head *
in argument type to bitmap.
* tree-ssa-loop-im.cc (memory_accesses): Change type of 3 members
from vec<bitmap_head> to vec<bitmap_head_pod>.
* tree-tailcall.cc (live_vars_vec): Change type from vec<bitmap_head>
to vec<bitmap_head_pod>.
* cfganal.cc (compute_dominance_frontiers, compute_idf): Change
argument types from bitmap_head * to bitmap.
* rtl-ssa/internals.h (function_info::bb_phi_info): Change regs
member type from bitmap_head to bitmap_head_pod.
* rtl-ssa/blocks.cc (function_info::place_phis): Change frontiers
and unfiltered variable types from auto_vec<bitmap_head> to
auto_vec<bitmap_head_pod>.
* tree-ssa-pre.cc (class bitmap_set): Change expressions and values
member types from bitmap_head to bitmap_head_pod.
* tree-inline.cc (add_clobbers_to_eh_landing_pad): Change live
variable type from vec<bitmap_head> to vec<bitmap_head_pod>.
* cfganal.h (class control_dependences): Change control_dependence_map
member type from vec<bitmap_head> to vec<bitmap_head_pod>.
(compute_dominance_frontiers, compute_idf): Change argument types from
class bitmap_head * to bitmap.
--- gcc/coretypes.h.jj 2023-08-24 15:37:28.233424065 +0200
+++ gcc/coretypes.h 2023-09-27 14:17:00.764453202 +0200
@@ -47,9 +47,10 @@ typedef int64_t gcov_type;
typedef uint64_t gcov_type_unsigned;
struct bitmap_obstack;
+struct bitmap_head_pod;
class bitmap_head;
-typedef class bitmap_head *bitmap;
-typedef const class bitmap_head *const_bitmap;
+typedef class bitmap_head_pod *bitmap;
+typedef const class bitmap_head_pod *const_bitmap;
struct simple_bitmap_def;
typedef struct simple_bitmap_def *sbitmap;
typedef const struct simple_bitmap_def *const_sbitmap;
--- gcc/bitmap.h.jj 2023-04-19 09:33:59.141354388 +0200
+++ gcc/bitmap.h 2023-09-27 15:13:45.700647295 +0200
@@ -293,7 +293,7 @@ typedef unsigned long BITMAP_WORD;
/* Obstack for allocating bitmaps and elements from. */
struct bitmap_obstack {
struct bitmap_element *elements;
- bitmap_head *heads;
+ bitmap_head_pod *heads;
struct obstack obstack;
};
@@ -323,15 +323,16 @@ struct GTY((chain_next ("%h.next"))) bit
};
/* Head of bitmap linked list. The 'current' member points to something
- already pointed to by the chain started by first, so GTY((skip)) it. */
+ already pointed to by the chain started by first, so GTY((skip)) it.
+ desc("0"), tag("0") is just to make gengtype happy, for GC there is
+ no difference between bitmap_head_pod and bitmap_head types. */
-class GTY(()) bitmap_head {
-public:
+struct GTY((desc("0"), tag("0"))) bitmap_head_pod {
static bitmap_obstack crashme;
- /* Poison obstack to not make it not a valid initialized GC bitmap. */
- CONSTEXPR bitmap_head()
- : indx (0), tree_form (false), padding (0), alloc_descriptor (0), first (NULL),
- current (NULL), obstack (&crashme)
+ bitmap_head_pod () = default;
+ CONSTEXPR bitmap_head_pod (unsigned int n, bool t, unsigned p, unsigned a)
+ : indx (n), tree_form (t), padding (p), alloc_descriptor (a),
+ first (NULL), current (NULL), obstack (&crashme)
{}
/* Index of last element looked at. */
unsigned int indx;
@@ -362,6 +363,13 @@ public:
}
};
+class GTY(()) bitmap_head : public bitmap_head_pod {
+public:
+ /* Poison obstack to not make it not a valid initialized GC bitmap. */
+ CONSTEXPR bitmap_head() : bitmap_head_pod (0, false, 0, 0)
+ {}
+};
+
/* Global data */
extern bitmap_element bitmap_zero_bits; /* Zero bitmap element */
extern bitmap_obstack bitmap_default_obstack; /* Default bitmap obstack */
--- gcc/bitmap.cc.jj 2023-04-19 09:33:59.141354388 +0200
+++ gcc/bitmap.cc 2023-09-27 19:48:33.347080275 +0200
@@ -28,7 +28,7 @@ mem_alloc_description<bitmap_usage> bitm
/* Static zero-initialized bitmap obstack used for default initialization
of bitmap_head. */
-bitmap_obstack bitmap_head::crashme;
+bitmap_obstack bitmap_head_pod::crashme;
static bitmap_element *bitmap_tree_listify_from (bitmap, bitmap_element *);
@@ -2849,13 +2849,13 @@ dump_bitmap_statistics (void)
}
DEBUG_FUNCTION void
-debug (const bitmap_head &ref)
+debug (const bitmap_head_pod &ref)
{
dump_bitmap (stderr, &ref);
}
DEBUG_FUNCTION void
-debug (const bitmap_head *ptr)
+debug (const bitmap_head_pod *ptr)
{
if (ptr)
debug (*ptr);
@@ -2866,17 +2866,17 @@ debug (const bitmap_head *ptr)
DEBUG_FUNCTION void
debug (const auto_bitmap &ref)
{
- debug ((const bitmap_head &) ref);
+ debug ((const bitmap_head_pod &) ref);
}
DEBUG_FUNCTION void
debug (const auto_bitmap *ptr)
{
- debug ((const bitmap_head *) ptr);
+ debug ((const bitmap_head_pod *) ptr);
}
void
-bitmap_head::dump ()
+bitmap_head_pod::dump ()
{
debug (this);
}
--- gcc/tree-ssa-live.h.jj 2023-09-06 17:28:39.333779470 +0200
+++ gcc/tree-ssa-live.h 2023-09-27 19:45:07.104895327 +0200
@@ -271,10 +271,11 @@ extern void debug (tree_live_info_d *ptr
extern void dump_live_info (FILE *, tree_live_info_p, int);
typedef hash_map<int_hash <unsigned int, -1U>, unsigned int> live_vars_map;
-extern vec<bitmap_head> compute_live_vars (struct function *, live_vars_map *);
-extern bitmap live_vars_at_stmt (vec<bitmap_head> &, live_vars_map *,
+extern vec<bitmap_head_pod> compute_live_vars (struct function *,
+ live_vars_map *);
+extern bitmap live_vars_at_stmt (vec<bitmap_head_pod> &, live_vars_map *,
gimple *);
-extern void destroy_live_vars (vec<bitmap_head> &);
+extern void destroy_live_vars (vec<bitmap_head_pod> &);
/* Return TRUE if P is marked as a global in LIVE. */
--- gcc/tree-ssa-live.cc.jj 2023-09-06 17:28:39.333779470 +0200
+++ gcc/tree-ssa-live.cc 2023-09-27 19:44:44.875198746 +0200
@@ -1281,7 +1281,7 @@ struct compute_live_vars_data {
/* Vector of bitmaps for live vars indices at the end of basic blocks,
indexed by bb->index. ACTIVE[ENTRY_BLOCK] must be empty bitmap,
ACTIVE[EXIT_BLOCK] is used for STOP_AFTER. */
- vec<bitmap_head> active;
+ vec<bitmap_head_pod> active;
/* Work bitmap of currently live variables. */
bitmap work;
/* Set of interesting variables. Variables with uids not in this
@@ -1345,10 +1345,10 @@ compute_live_vars_1 (basic_block bb, com
indexes of automatic variables VARS, compute which of those variables are
(might be) live at the end of each basic block. */
-vec<bitmap_head>
+vec<bitmap_head_pod>
compute_live_vars (struct function *fn, live_vars_map *vars)
{
- vec<bitmap_head> active;
+ vec<bitmap_head_pod> active;
/* We approximate the live range of a stack variable by taking the first
mention of its name as starting point(s), and by the end-of-scope
@@ -1395,7 +1395,7 @@ compute_live_vars (struct function *fn,
live after the STOP_AFTER statement and return that bitmap. */
bitmap
-live_vars_at_stmt (vec<bitmap_head> &active, live_vars_map *vars,
+live_vars_at_stmt (vec<bitmap_head_pod> &active, live_vars_map *vars,
gimple *stop_after)
{
bitmap work = BITMAP_ALLOC (NULL);
@@ -1408,7 +1408,7 @@ live_vars_at_stmt (vec<bitmap_head> &act
/* Destroy what compute_live_vars has returned when it is no longer needed. */
void
-destroy_live_vars (vec<bitmap_head> &active)
+destroy_live_vars (vec<bitmap_head_pod> &active)
{
unsigned len = active.length ();
for (unsigned i = 0; i < len; i++)
--- gcc/sel-sched-ir.h.jj 2023-02-28 11:29:22.291806607 +0100
+++ gcc/sel-sched-ir.h 2023-09-27 15:30:48.280695816 +0200
@@ -951,7 +951,7 @@ extern vec<sel_region_bb_info_def> sel_r
#define BB_AV_SET_VALID_P(BB) (BB_AV_LEVEL (BB) == global_level)
/* Used in bb_in_ebb_p. */
-extern bitmap_head *forced_ebb_heads;
+extern bitmap forced_ebb_heads;
/* The loop nest being pipelined. */
extern class loop *current_loop_nest;
--- gcc/gimple-range-path.cc.jj 2023-04-27 10:17:45.874494495 +0200
+++ gcc/gimple-range-path.cc 2023-09-27 15:27:16.326603893 +0200
@@ -38,7 +38,7 @@ along with GCC; see the file COPYING3.
path_range_query::path_range_query (gimple_ranger &ranger,
const vec<basic_block> &path,
- const bitmap_head *dependencies,
+ const_bitmap dependencies,
bool resolve)
: m_cache (),
m_ranger (ranger),
@@ -192,7 +192,7 @@ path_range_query::unreachable_path_p ()
void
path_range_query::reset_path (const vec<basic_block> &path,
- const bitmap_head *dependencies)
+ const_bitmap dependencies)
{
gcc_checking_assert (path.length () > 1);
m_path = path.copy ();
@@ -549,7 +549,7 @@ path_range_query::compute_exit_dependenc
// calculated from the final conditional in the path.
void
-path_range_query::compute_ranges (const bitmap_head *dependencies)
+path_range_query::compute_ranges (const_bitmap dependencies)
{
if (DEBUG_SOLVER)
fprintf (dump_file, "\n==============================================\n");
--- gcc/gimple-range-path.h.jj 2023-04-27 10:17:45.874494495 +0200
+++ gcc/gimple-range-path.h 2023-09-27 15:26:52.980924197 +0200
@@ -34,11 +34,11 @@ class path_range_query : public range_qu
public:
path_range_query (class gimple_ranger &ranger,
const vec<basic_block> &path,
- const bitmap_head *dependencies = NULL,
+ const_bitmap dependencies = NULL,
bool resolve = true);
path_range_query (gimple_ranger &ranger, bool resolve = true);
virtual ~path_range_query ();
- void reset_path (const vec<basic_block> &, const bitmap_head *dependencies);
+ void reset_path (const vec<basic_block> &, const_bitmap dependencies);
bool range_of_expr (vrange &r, tree name, gimple * = NULL) override;
bool range_of_stmt (vrange &r, gimple *, tree name = NULL) override;
bool unreachable_path_p ();
@@ -47,8 +47,8 @@ public:
private:
bool internal_range_of_expr (vrange &r, tree name, gimple *);
- void compute_ranges (const bitmap_head *dependencies);
- void compute_exit_dependencies (bitmap_head *dependencies);
+ void compute_ranges (const_bitmap dependencies);
+ void compute_exit_dependencies (bitmap dependencies);
bool defined_outside_path (tree name);
void range_on_path_entry (vrange &r, tree name);
path_oracle *get_path_oracle () { return (path_oracle *)m_oracle; }
--- gcc/tree-ssa-loop-im.cc.jj 2023-09-27 13:40:52.034332877 +0200
+++ gcc/tree-ssa-loop-im.cc 2023-09-27 14:21:18.486901776 +0200
@@ -241,13 +241,13 @@ static struct
vec<im_mem_ref *> refs_list;
/* The set of memory references accessed in each loop. */
- vec<bitmap_head> refs_loaded_in_loop;
+ vec<bitmap_head_pod> refs_loaded_in_loop;
/* The set of memory references stored in each loop. */
- vec<bitmap_head> refs_stored_in_loop;
+ vec<bitmap_head_pod> refs_stored_in_loop;
/* The set of memory references stored in each loop, including subloops . */
- vec<bitmap_head> all_refs_stored_in_loop;
+ vec<bitmap_head_pod> all_refs_stored_in_loop;
/* Cache for expanding memory addresses. */
hash_map<tree, name_expansion *> *ttae_cache;
--- gcc/tree-tailcall.cc.jj 2023-04-24 10:35:04.913938226 +0200
+++ gcc/tree-tailcall.cc 2023-09-27 19:47:11.487197594 +0200
@@ -405,7 +405,7 @@ propagate_through_phis (tree var, edge e
/* Argument for compute_live_vars/live_vars_at_stmt and what compute_live_vars
returns. Computed lazily, but just once for the function. */
static live_vars_map *live_vars;
-static vec<bitmap_head> live_vars_vec;
+static vec<bitmap_head_pod> live_vars_vec;
/* Finds tailcalls falling into basic block BB. The list of found tailcalls is
added to the start of RET. */
--- gcc/cfganal.cc.jj 2023-04-22 10:23:40.461614962 +0200
+++ gcc/cfganal.cc 2023-09-27 15:29:14.280985521 +0200
@@ -1636,7 +1636,7 @@ dfs_enumerate_from (basic_block bb, int
*/
void
-compute_dominance_frontiers (bitmap_head *frontiers)
+compute_dominance_frontiers (bitmap frontiers)
{
timevar_push (TV_DOM_FRONTIERS);
@@ -1677,7 +1677,7 @@ compute_dominance_frontiers (bitmap_head
allocated for the return value. */
bitmap
-compute_idf (bitmap def_blocks, bitmap_head *dfs)
+compute_idf (bitmap def_blocks, bitmap dfs)
{
bitmap_iterator bi;
unsigned bb_index, i;
--- gcc/rtl-ssa/internals.h.jj 2023-01-16 11:52:16.250731603 +0100
+++ gcc/rtl-ssa/internals.h 2023-09-27 14:24:21.961373493 +0200
@@ -25,7 +25,7 @@ class function_info::bb_phi_info
{
public:
// The set of registers that need phi nodes.
- bitmap_head regs;
+ bitmap_head_pod regs;
// The number of registers in REGS.
unsigned int num_phis;
--- gcc/rtl-ssa/blocks.cc.jj 2023-01-16 11:52:16.249731618 +0100
+++ gcc/rtl-ssa/blocks.cc 2023-09-27 14:25:08.113737510 +0200
@@ -613,7 +613,7 @@ function_info::place_phis (build_info &b
unsigned int num_bb_indices = last_basic_block_for_fn (m_fn);
// Calculate dominance frontiers.
- auto_vec<bitmap_head> frontiers;
+ auto_vec<bitmap_head_pod> frontiers;
frontiers.safe_grow (num_bb_indices);
for (unsigned int i = 0; i < num_bb_indices; ++i)
bitmap_initialize (&frontiers[i], &bitmap_default_obstack);
@@ -625,7 +625,7 @@ function_info::place_phis (build_info &b
// PENDING as a staging area: registers in PENDING need phi nodes if
// they are live on entry to the corresponding block, but do not need
// phi nodes otherwise.
- auto_vec<bitmap_head> unfiltered;
+ auto_vec<bitmap_head_pod> unfiltered;
unfiltered.safe_grow (num_bb_indices);
for (unsigned int i = 0; i < num_bb_indices; ++i)
bitmap_initialize (&unfiltered[i], &bitmap_default_obstack);
--- gcc/tree-ssa-pre.cc.jj 2023-08-18 09:33:45.935664082 +0200
+++ gcc/tree-ssa-pre.cc 2023-09-27 19:46:23.099858050 +0200
@@ -484,8 +484,8 @@ get_or_alloc_expr_for_reference (vn_refe
typedef class bitmap_set
{
public:
- bitmap_head expressions;
- bitmap_head values;
+ bitmap_head_pod expressions;
+ bitmap_head_pod values;
} *bitmap_set_t;
#define FOR_EACH_EXPR_ID_IN_SET(set, id, bi) \
--- gcc/tree-inline.cc.jj 2023-08-28 10:33:11.782178374 +0200
+++ gcc/tree-inline.cc 2023-09-27 19:47:40.719798594 +0200
@@ -2475,7 +2475,7 @@ add_clobbers_to_eh_landing_pad (copy_bod
if (vars == NULL)
return;
- vec<bitmap_head> live = compute_live_vars (id->src_cfun, vars);
+ vec<bitmap_head_pod> live = compute_live_vars (id->src_cfun, vars);
FOR_EACH_VEC_SAFE_ELT (id->src_cfun->local_decls, i, var)
if (VAR_P (var))
{
--- gcc/cfganal.h.jj 2023-04-22 10:23:40.472614801 +0200
+++ gcc/cfganal.h 2023-09-27 15:29:39.712636592 +0200
@@ -44,7 +44,7 @@ private:
void set_control_dependence_map_bit (basic_block, int);
void clear_control_dependence_bitmap (basic_block);
void find_control_dependence (int);
- vec<bitmap_head> control_dependence_map;
+ vec<bitmap_head_pod> control_dependence_map;
vec<std::pair<int, int> > m_el;
bitmap_obstack m_bitmaps;
};
@@ -77,8 +77,8 @@ extern int rev_post_order_and_mark_dfs_b
extern int dfs_enumerate_from (basic_block, int,
bool (*)(const_basic_block, const void *),
basic_block *, int, const void *);
-extern void compute_dominance_frontiers (class bitmap_head *);
-extern bitmap compute_idf (bitmap, class bitmap_head *);
+extern void compute_dominance_frontiers (bitmap);
+extern bitmap compute_idf (bitmap, bitmap);
extern void bitmap_intersection_of_succs (sbitmap, sbitmap *, basic_block);
extern void bitmap_intersection_of_preds (sbitmap, sbitmap *, basic_block);
extern void bitmap_union_of_succs (sbitmap, sbitmap *, basic_block);
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bitmap: Introduce bitmap_head_pod
2023-09-28 9:12 [PATCH] bitmap: Introduce bitmap_head_pod Jakub Jelinek
@ 2023-09-28 9:29 ` Richard Biener
2023-09-28 10:29 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2023-09-28 9:29 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Sandiford, gcc-patches
On Thu, 28 Sep 2023, Jakub Jelinek wrote:
> Hi!
>
> The following patch splits the bitmap_head class into a POD
> struct bitmap_head_pod and bitmap_head derived from it with non-trivial
> default constexpr constructor. Most code should keep using bitmap_head
> as before, bitmap_head_pod is there just for the cases where we want to
> embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
> and in a loop bitmap_initialize it afterwards (to avoid having to
> {quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
> The patch is larger than I hoped, because e.g. some code just used bitmap
> and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK if there are no comments indicating otherwise.
Thanks,
Richard.
> 2023-09-28 Jakub Jelinek <jakub@redhat.com>
>
> * coretypes.h (struct bitmap_head_pod): New forward declaration.
> (bitmap, const_bitmap): Replace bitmap_head with bitmap_head_pod
> in typedefs.
> * bitmap.h (struct bitmap_obstack): Change heads element type to
> bitmap_head_pod *.
> (struct bitmap_head_pod): New type.
> (class bitmap_head): Changed into derived class from bitmap_head_pod
> with a non-trivial constexpr default ctor.
> * bitmap.cc (bitmap_head::crashme): Remove.
> (bitmap_head_pod::crashme): New static data member.
> (debug): Change bitmap_head to bitmap_head_pod.
> (bitmap_head::dump): Renamed to ...
> (bitmap_head_pod::dump): ... this.
> * tree-ssa-live.h (compute_live_vars, live_vars_at_stmt,
> destroy_live_vars): Change vec<bitmap_head> to vec<bitmap_head_pod>.
> * tree-ssa-live.cc (struct compute_live_vars_data): Change active
> member type to vec<bitmap_head_pod>.
> (compute_live_vars, live_vars_at_stmt, destroy_live_vars): Change
> vec<bitmap_head> to vec<bitmap_head_pod>.
> * sel-sched-ir.h (forced_ebb_heads): Change type to bitmap.
> * gimple-range-path.cc (path_range_query::path_range_query,
> path_range_query::reset_path, path_range_query::compute_ranges):
> Change const bitmap_head * in argument types to const_bitmap.
> * gimple-range-path.h (path_range_query::path_range_query,
> path_range_query::reset_path, path_range_query::compute_ranges):
> Likewise.
> (path_range_query::compute_exit_dependencies): Change bitmap_head *
> in argument type to bitmap.
> * tree-ssa-loop-im.cc (memory_accesses): Change type of 3 members
> from vec<bitmap_head> to vec<bitmap_head_pod>.
> * tree-tailcall.cc (live_vars_vec): Change type from vec<bitmap_head>
> to vec<bitmap_head_pod>.
> * cfganal.cc (compute_dominance_frontiers, compute_idf): Change
> argument types from bitmap_head * to bitmap.
> * rtl-ssa/internals.h (function_info::bb_phi_info): Change regs
> member type from bitmap_head to bitmap_head_pod.
> * rtl-ssa/blocks.cc (function_info::place_phis): Change frontiers
> and unfiltered variable types from auto_vec<bitmap_head> to
> auto_vec<bitmap_head_pod>.
> * tree-ssa-pre.cc (class bitmap_set): Change expressions and values
> member types from bitmap_head to bitmap_head_pod.
> * tree-inline.cc (add_clobbers_to_eh_landing_pad): Change live
> variable type from vec<bitmap_head> to vec<bitmap_head_pod>.
> * cfganal.h (class control_dependences): Change control_dependence_map
> member type from vec<bitmap_head> to vec<bitmap_head_pod>.
> (compute_dominance_frontiers, compute_idf): Change argument types from
> class bitmap_head * to bitmap.
>
> --- gcc/coretypes.h.jj 2023-08-24 15:37:28.233424065 +0200
> +++ gcc/coretypes.h 2023-09-27 14:17:00.764453202 +0200
> @@ -47,9 +47,10 @@ typedef int64_t gcov_type;
> typedef uint64_t gcov_type_unsigned;
>
> struct bitmap_obstack;
> +struct bitmap_head_pod;
> class bitmap_head;
> -typedef class bitmap_head *bitmap;
> -typedef const class bitmap_head *const_bitmap;
> +typedef class bitmap_head_pod *bitmap;
> +typedef const class bitmap_head_pod *const_bitmap;
> struct simple_bitmap_def;
> typedef struct simple_bitmap_def *sbitmap;
> typedef const struct simple_bitmap_def *const_sbitmap;
> --- gcc/bitmap.h.jj 2023-04-19 09:33:59.141354388 +0200
> +++ gcc/bitmap.h 2023-09-27 15:13:45.700647295 +0200
> @@ -293,7 +293,7 @@ typedef unsigned long BITMAP_WORD;
> /* Obstack for allocating bitmaps and elements from. */
> struct bitmap_obstack {
> struct bitmap_element *elements;
> - bitmap_head *heads;
> + bitmap_head_pod *heads;
> struct obstack obstack;
> };
>
> @@ -323,15 +323,16 @@ struct GTY((chain_next ("%h.next"))) bit
> };
>
> /* Head of bitmap linked list. The 'current' member points to something
> - already pointed to by the chain started by first, so GTY((skip)) it. */
> + already pointed to by the chain started by first, so GTY((skip)) it.
> + desc("0"), tag("0") is just to make gengtype happy, for GC there is
> + no difference between bitmap_head_pod and bitmap_head types. */
>
> -class GTY(()) bitmap_head {
> -public:
> +struct GTY((desc("0"), tag("0"))) bitmap_head_pod {
> static bitmap_obstack crashme;
> - /* Poison obstack to not make it not a valid initialized GC bitmap. */
> - CONSTEXPR bitmap_head()
> - : indx (0), tree_form (false), padding (0), alloc_descriptor (0), first (NULL),
> - current (NULL), obstack (&crashme)
> + bitmap_head_pod () = default;
> + CONSTEXPR bitmap_head_pod (unsigned int n, bool t, unsigned p, unsigned a)
> + : indx (n), tree_form (t), padding (p), alloc_descriptor (a),
> + first (NULL), current (NULL), obstack (&crashme)
> {}
> /* Index of last element looked at. */
> unsigned int indx;
> @@ -362,6 +363,13 @@ public:
> }
> };
>
> +class GTY(()) bitmap_head : public bitmap_head_pod {
> +public:
> + /* Poison obstack to not make it not a valid initialized GC bitmap. */
> + CONSTEXPR bitmap_head() : bitmap_head_pod (0, false, 0, 0)
> + {}
> +};
> +
> /* Global data */
> extern bitmap_element bitmap_zero_bits; /* Zero bitmap element */
> extern bitmap_obstack bitmap_default_obstack; /* Default bitmap obstack */
> --- gcc/bitmap.cc.jj 2023-04-19 09:33:59.141354388 +0200
> +++ gcc/bitmap.cc 2023-09-27 19:48:33.347080275 +0200
> @@ -28,7 +28,7 @@ mem_alloc_description<bitmap_usage> bitm
>
> /* Static zero-initialized bitmap obstack used for default initialization
> of bitmap_head. */
> -bitmap_obstack bitmap_head::crashme;
> +bitmap_obstack bitmap_head_pod::crashme;
>
> static bitmap_element *bitmap_tree_listify_from (bitmap, bitmap_element *);
>
> @@ -2849,13 +2849,13 @@ dump_bitmap_statistics (void)
> }
>
> DEBUG_FUNCTION void
> -debug (const bitmap_head &ref)
> +debug (const bitmap_head_pod &ref)
> {
> dump_bitmap (stderr, &ref);
> }
>
> DEBUG_FUNCTION void
> -debug (const bitmap_head *ptr)
> +debug (const bitmap_head_pod *ptr)
> {
> if (ptr)
> debug (*ptr);
> @@ -2866,17 +2866,17 @@ debug (const bitmap_head *ptr)
> DEBUG_FUNCTION void
> debug (const auto_bitmap &ref)
> {
> - debug ((const bitmap_head &) ref);
> + debug ((const bitmap_head_pod &) ref);
> }
>
> DEBUG_FUNCTION void
> debug (const auto_bitmap *ptr)
> {
> - debug ((const bitmap_head *) ptr);
> + debug ((const bitmap_head_pod *) ptr);
> }
>
> void
> -bitmap_head::dump ()
> +bitmap_head_pod::dump ()
> {
> debug (this);
> }
> --- gcc/tree-ssa-live.h.jj 2023-09-06 17:28:39.333779470 +0200
> +++ gcc/tree-ssa-live.h 2023-09-27 19:45:07.104895327 +0200
> @@ -271,10 +271,11 @@ extern void debug (tree_live_info_d *ptr
> extern void dump_live_info (FILE *, tree_live_info_p, int);
>
> typedef hash_map<int_hash <unsigned int, -1U>, unsigned int> live_vars_map;
> -extern vec<bitmap_head> compute_live_vars (struct function *, live_vars_map *);
> -extern bitmap live_vars_at_stmt (vec<bitmap_head> &, live_vars_map *,
> +extern vec<bitmap_head_pod> compute_live_vars (struct function *,
> + live_vars_map *);
> +extern bitmap live_vars_at_stmt (vec<bitmap_head_pod> &, live_vars_map *,
> gimple *);
> -extern void destroy_live_vars (vec<bitmap_head> &);
> +extern void destroy_live_vars (vec<bitmap_head_pod> &);
>
> /* Return TRUE if P is marked as a global in LIVE. */
>
> --- gcc/tree-ssa-live.cc.jj 2023-09-06 17:28:39.333779470 +0200
> +++ gcc/tree-ssa-live.cc 2023-09-27 19:44:44.875198746 +0200
> @@ -1281,7 +1281,7 @@ struct compute_live_vars_data {
> /* Vector of bitmaps for live vars indices at the end of basic blocks,
> indexed by bb->index. ACTIVE[ENTRY_BLOCK] must be empty bitmap,
> ACTIVE[EXIT_BLOCK] is used for STOP_AFTER. */
> - vec<bitmap_head> active;
> + vec<bitmap_head_pod> active;
> /* Work bitmap of currently live variables. */
> bitmap work;
> /* Set of interesting variables. Variables with uids not in this
> @@ -1345,10 +1345,10 @@ compute_live_vars_1 (basic_block bb, com
> indexes of automatic variables VARS, compute which of those variables are
> (might be) live at the end of each basic block. */
>
> -vec<bitmap_head>
> +vec<bitmap_head_pod>
> compute_live_vars (struct function *fn, live_vars_map *vars)
> {
> - vec<bitmap_head> active;
> + vec<bitmap_head_pod> active;
>
> /* We approximate the live range of a stack variable by taking the first
> mention of its name as starting point(s), and by the end-of-scope
> @@ -1395,7 +1395,7 @@ compute_live_vars (struct function *fn,
> live after the STOP_AFTER statement and return that bitmap. */
>
> bitmap
> -live_vars_at_stmt (vec<bitmap_head> &active, live_vars_map *vars,
> +live_vars_at_stmt (vec<bitmap_head_pod> &active, live_vars_map *vars,
> gimple *stop_after)
> {
> bitmap work = BITMAP_ALLOC (NULL);
> @@ -1408,7 +1408,7 @@ live_vars_at_stmt (vec<bitmap_head> &act
> /* Destroy what compute_live_vars has returned when it is no longer needed. */
>
> void
> -destroy_live_vars (vec<bitmap_head> &active)
> +destroy_live_vars (vec<bitmap_head_pod> &active)
> {
> unsigned len = active.length ();
> for (unsigned i = 0; i < len; i++)
> --- gcc/sel-sched-ir.h.jj 2023-02-28 11:29:22.291806607 +0100
> +++ gcc/sel-sched-ir.h 2023-09-27 15:30:48.280695816 +0200
> @@ -951,7 +951,7 @@ extern vec<sel_region_bb_info_def> sel_r
> #define BB_AV_SET_VALID_P(BB) (BB_AV_LEVEL (BB) == global_level)
>
> /* Used in bb_in_ebb_p. */
> -extern bitmap_head *forced_ebb_heads;
> +extern bitmap forced_ebb_heads;
>
> /* The loop nest being pipelined. */
> extern class loop *current_loop_nest;
> --- gcc/gimple-range-path.cc.jj 2023-04-27 10:17:45.874494495 +0200
> +++ gcc/gimple-range-path.cc 2023-09-27 15:27:16.326603893 +0200
> @@ -38,7 +38,7 @@ along with GCC; see the file COPYING3.
>
> path_range_query::path_range_query (gimple_ranger &ranger,
> const vec<basic_block> &path,
> - const bitmap_head *dependencies,
> + const_bitmap dependencies,
> bool resolve)
> : m_cache (),
> m_ranger (ranger),
> @@ -192,7 +192,7 @@ path_range_query::unreachable_path_p ()
>
> void
> path_range_query::reset_path (const vec<basic_block> &path,
> - const bitmap_head *dependencies)
> + const_bitmap dependencies)
> {
> gcc_checking_assert (path.length () > 1);
> m_path = path.copy ();
> @@ -549,7 +549,7 @@ path_range_query::compute_exit_dependenc
> // calculated from the final conditional in the path.
>
> void
> -path_range_query::compute_ranges (const bitmap_head *dependencies)
> +path_range_query::compute_ranges (const_bitmap dependencies)
> {
> if (DEBUG_SOLVER)
> fprintf (dump_file, "\n==============================================\n");
> --- gcc/gimple-range-path.h.jj 2023-04-27 10:17:45.874494495 +0200
> +++ gcc/gimple-range-path.h 2023-09-27 15:26:52.980924197 +0200
> @@ -34,11 +34,11 @@ class path_range_query : public range_qu
> public:
> path_range_query (class gimple_ranger &ranger,
> const vec<basic_block> &path,
> - const bitmap_head *dependencies = NULL,
> + const_bitmap dependencies = NULL,
> bool resolve = true);
> path_range_query (gimple_ranger &ranger, bool resolve = true);
> virtual ~path_range_query ();
> - void reset_path (const vec<basic_block> &, const bitmap_head *dependencies);
> + void reset_path (const vec<basic_block> &, const_bitmap dependencies);
> bool range_of_expr (vrange &r, tree name, gimple * = NULL) override;
> bool range_of_stmt (vrange &r, gimple *, tree name = NULL) override;
> bool unreachable_path_p ();
> @@ -47,8 +47,8 @@ public:
>
> private:
> bool internal_range_of_expr (vrange &r, tree name, gimple *);
> - void compute_ranges (const bitmap_head *dependencies);
> - void compute_exit_dependencies (bitmap_head *dependencies);
> + void compute_ranges (const_bitmap dependencies);
> + void compute_exit_dependencies (bitmap dependencies);
> bool defined_outside_path (tree name);
> void range_on_path_entry (vrange &r, tree name);
> path_oracle *get_path_oracle () { return (path_oracle *)m_oracle; }
> --- gcc/tree-ssa-loop-im.cc.jj 2023-09-27 13:40:52.034332877 +0200
> +++ gcc/tree-ssa-loop-im.cc 2023-09-27 14:21:18.486901776 +0200
> @@ -241,13 +241,13 @@ static struct
> vec<im_mem_ref *> refs_list;
>
> /* The set of memory references accessed in each loop. */
> - vec<bitmap_head> refs_loaded_in_loop;
> + vec<bitmap_head_pod> refs_loaded_in_loop;
>
> /* The set of memory references stored in each loop. */
> - vec<bitmap_head> refs_stored_in_loop;
> + vec<bitmap_head_pod> refs_stored_in_loop;
>
> /* The set of memory references stored in each loop, including subloops . */
> - vec<bitmap_head> all_refs_stored_in_loop;
> + vec<bitmap_head_pod> all_refs_stored_in_loop;
>
> /* Cache for expanding memory addresses. */
> hash_map<tree, name_expansion *> *ttae_cache;
> --- gcc/tree-tailcall.cc.jj 2023-04-24 10:35:04.913938226 +0200
> +++ gcc/tree-tailcall.cc 2023-09-27 19:47:11.487197594 +0200
> @@ -405,7 +405,7 @@ propagate_through_phis (tree var, edge e
> /* Argument for compute_live_vars/live_vars_at_stmt and what compute_live_vars
> returns. Computed lazily, but just once for the function. */
> static live_vars_map *live_vars;
> -static vec<bitmap_head> live_vars_vec;
> +static vec<bitmap_head_pod> live_vars_vec;
>
> /* Finds tailcalls falling into basic block BB. The list of found tailcalls is
> added to the start of RET. */
> --- gcc/cfganal.cc.jj 2023-04-22 10:23:40.461614962 +0200
> +++ gcc/cfganal.cc 2023-09-27 15:29:14.280985521 +0200
> @@ -1636,7 +1636,7 @@ dfs_enumerate_from (basic_block bb, int
> */
>
> void
> -compute_dominance_frontiers (bitmap_head *frontiers)
> +compute_dominance_frontiers (bitmap frontiers)
> {
> timevar_push (TV_DOM_FRONTIERS);
>
> @@ -1677,7 +1677,7 @@ compute_dominance_frontiers (bitmap_head
> allocated for the return value. */
>
> bitmap
> -compute_idf (bitmap def_blocks, bitmap_head *dfs)
> +compute_idf (bitmap def_blocks, bitmap dfs)
> {
> bitmap_iterator bi;
> unsigned bb_index, i;
> --- gcc/rtl-ssa/internals.h.jj 2023-01-16 11:52:16.250731603 +0100
> +++ gcc/rtl-ssa/internals.h 2023-09-27 14:24:21.961373493 +0200
> @@ -25,7 +25,7 @@ class function_info::bb_phi_info
> {
> public:
> // The set of registers that need phi nodes.
> - bitmap_head regs;
> + bitmap_head_pod regs;
>
> // The number of registers in REGS.
> unsigned int num_phis;
> --- gcc/rtl-ssa/blocks.cc.jj 2023-01-16 11:52:16.249731618 +0100
> +++ gcc/rtl-ssa/blocks.cc 2023-09-27 14:25:08.113737510 +0200
> @@ -613,7 +613,7 @@ function_info::place_phis (build_info &b
> unsigned int num_bb_indices = last_basic_block_for_fn (m_fn);
>
> // Calculate dominance frontiers.
> - auto_vec<bitmap_head> frontiers;
> + auto_vec<bitmap_head_pod> frontiers;
> frontiers.safe_grow (num_bb_indices);
> for (unsigned int i = 0; i < num_bb_indices; ++i)
> bitmap_initialize (&frontiers[i], &bitmap_default_obstack);
> @@ -625,7 +625,7 @@ function_info::place_phis (build_info &b
> // PENDING as a staging area: registers in PENDING need phi nodes if
> // they are live on entry to the corresponding block, but do not need
> // phi nodes otherwise.
> - auto_vec<bitmap_head> unfiltered;
> + auto_vec<bitmap_head_pod> unfiltered;
> unfiltered.safe_grow (num_bb_indices);
> for (unsigned int i = 0; i < num_bb_indices; ++i)
> bitmap_initialize (&unfiltered[i], &bitmap_default_obstack);
> --- gcc/tree-ssa-pre.cc.jj 2023-08-18 09:33:45.935664082 +0200
> +++ gcc/tree-ssa-pre.cc 2023-09-27 19:46:23.099858050 +0200
> @@ -484,8 +484,8 @@ get_or_alloc_expr_for_reference (vn_refe
> typedef class bitmap_set
> {
> public:
> - bitmap_head expressions;
> - bitmap_head values;
> + bitmap_head_pod expressions;
> + bitmap_head_pod values;
> } *bitmap_set_t;
>
> #define FOR_EACH_EXPR_ID_IN_SET(set, id, bi) \
> --- gcc/tree-inline.cc.jj 2023-08-28 10:33:11.782178374 +0200
> +++ gcc/tree-inline.cc 2023-09-27 19:47:40.719798594 +0200
> @@ -2475,7 +2475,7 @@ add_clobbers_to_eh_landing_pad (copy_bod
> if (vars == NULL)
> return;
>
> - vec<bitmap_head> live = compute_live_vars (id->src_cfun, vars);
> + vec<bitmap_head_pod> live = compute_live_vars (id->src_cfun, vars);
> FOR_EACH_VEC_SAFE_ELT (id->src_cfun->local_decls, i, var)
> if (VAR_P (var))
> {
> --- gcc/cfganal.h.jj 2023-04-22 10:23:40.472614801 +0200
> +++ gcc/cfganal.h 2023-09-27 15:29:39.712636592 +0200
> @@ -44,7 +44,7 @@ private:
> void set_control_dependence_map_bit (basic_block, int);
> void clear_control_dependence_bitmap (basic_block);
> void find_control_dependence (int);
> - vec<bitmap_head> control_dependence_map;
> + vec<bitmap_head_pod> control_dependence_map;
> vec<std::pair<int, int> > m_el;
> bitmap_obstack m_bitmaps;
> };
> @@ -77,8 +77,8 @@ extern int rev_post_order_and_mark_dfs_b
> extern int dfs_enumerate_from (basic_block, int,
> bool (*)(const_basic_block, const void *),
> basic_block *, int, const void *);
> -extern void compute_dominance_frontiers (class bitmap_head *);
> -extern bitmap compute_idf (bitmap, class bitmap_head *);
> +extern void compute_dominance_frontiers (bitmap);
> +extern bitmap compute_idf (bitmap, bitmap);
> extern void bitmap_intersection_of_succs (sbitmap, sbitmap *, basic_block);
> extern void bitmap_intersection_of_preds (sbitmap, sbitmap *, basic_block);
> extern void bitmap_union_of_succs (sbitmap, sbitmap *, basic_block);
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bitmap: Introduce bitmap_head_pod
2023-09-28 9:29 ` Richard Biener
@ 2023-09-28 10:29 ` Jakub Jelinek
2023-09-28 10:47 ` [PATCH] use *_grow_cleared rather than *_grow on vec<bitmap_head> Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2023-09-28 10:29 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Sandiford, gcc-patches
On Thu, Sep 28, 2023 at 09:29:31AM +0000, Richard Biener wrote:
> > The following patch splits the bitmap_head class into a POD
> > struct bitmap_head_pod and bitmap_head derived from it with non-trivial
> > default constexpr constructor. Most code should keep using bitmap_head
> > as before, bitmap_head_pod is there just for the cases where we want to
> > embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
> > and in a loop bitmap_initialize it afterwards (to avoid having to
> > {quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
> > The patch is larger than I hoped, because e.g. some code just used bitmap
> > and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> OK if there are no comments indicating otherwise.
A counter argument against this patch would be that it weakens the intent
to catch uses of uninitialized bitmaps for saving a few compile time cycles.
If one uses
bitmap_head var;
bitmap_initialize (&var, NULL);
etc., we spend those extra cycles to initialize it and nothing is told that
bitmap_initialize overwrites the whole var without ever using of any of its
elements, so DSE can't eliminate that. And in the vec case which prompted
this patch it was about
vec<bitmap_head> a;
a.create (n);
a.safe_grow (n); // vs. a.safe_grow_cleared (n);
for (int i = 0; i < n; ++i)
bitmap_initialize (&a[i], NULL);
When using bitmap_head_pod, one needs to ensure initialization without
help to catch such mistakes.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] use *_grow_cleared rather than *_grow on vec<bitmap_head>
2023-09-28 10:29 ` Jakub Jelinek
@ 2023-09-28 10:47 ` Jakub Jelinek
2023-09-28 11:54 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2023-09-28 10:47 UTC (permalink / raw)
To: Richard Biener, Richard Sandiford, gcc-patches
On Thu, Sep 28, 2023 at 12:29:15PM +0200, Jakub Jelinek wrote:
> On Thu, Sep 28, 2023 at 09:29:31AM +0000, Richard Biener wrote:
> > > The following patch splits the bitmap_head class into a POD
> > > struct bitmap_head_pod and bitmap_head derived from it with non-trivial
> > > default constexpr constructor. Most code should keep using bitmap_head
> > > as before, bitmap_head_pod is there just for the cases where we want to
> > > embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
> > > and in a loop bitmap_initialize it afterwards (to avoid having to
> > > {quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
> > > The patch is larger than I hoped, because e.g. some code just used bitmap
> > > and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > OK if there are no comments indicating otherwise.
>
> A counter argument against this patch would be that it weakens the intent
> to catch uses of uninitialized bitmaps for saving a few compile time cycles.
> If one uses
> bitmap_head var;
> bitmap_initialize (&var, NULL);
> etc., we spend those extra cycles to initialize it and nothing is told that
> bitmap_initialize overwrites the whole var without ever using of any of its
> elements, so DSE can't eliminate that. And in the vec case which prompted
> this patch it was about
> vec<bitmap_head> a;
> a.create (n);
> a.safe_grow (n); // vs. a.safe_grow_cleared (n);
> for (int i = 0; i < n; ++i)
> bitmap_initialize (&a[i], NULL);
> When using bitmap_head_pod, one needs to ensure initialization without
> help to catch such mistakes.
Here is the alternative patch which pays the small extra price while not
undermining the checking. Verified in all those places there is a loop
doing bitmap_initialize immediately afterwards or worst case a few lines
later.
With the static_assert uncommented, the remaining failures are poly_int
related (supposedly gone with Richard S.'s poly_int patch) and the
vect_unpromoted_value/ao_ref still unresolved cases.
2023-09-28 Jakub Jelinek <jakub@redhat.com>
* tree-ssa-loop-im.cc (tree_ssa_lim_initialize): Use quick_grow_cleared
instead of quick_grow on vec<bitmap_head> members.
* cfganal.cc (control_dependences::control_dependences): Likewise.
* rtl-ssa/blocks.cc (function_info::build_info::build_info): Likewise.
(function_info::place_phis): Use safe_grow_cleared instead of safe_grow
on auto_vec<bitmap_head> vars.
* tree-ssa-live.cc (compute_live_vars): Use quick_grow_cleared instead
of quick_grow on vec<bitmap_head> var.
--- gcc/tree-ssa-loop-im.cc.jj 2023-09-28 12:06:03.527974171 +0200
+++ gcc/tree-ssa-loop-im.cc 2023-09-28 12:38:07.028966742 +0200
@@ -3496,13 +3496,13 @@ tree_ssa_lim_initialize (bool store_moti
(mem_ref_alloc (NULL, 0, UNANALYZABLE_MEM_ID));
memory_accesses.refs_loaded_in_loop.create (number_of_loops (cfun));
- memory_accesses.refs_loaded_in_loop.quick_grow (number_of_loops (cfun));
+ memory_accesses.refs_loaded_in_loop.quick_grow_cleared (number_of_loops (cfun));
memory_accesses.refs_stored_in_loop.create (number_of_loops (cfun));
- memory_accesses.refs_stored_in_loop.quick_grow (number_of_loops (cfun));
+ memory_accesses.refs_stored_in_loop.quick_grow_cleared (number_of_loops (cfun));
if (store_motion)
{
memory_accesses.all_refs_stored_in_loop.create (number_of_loops (cfun));
- memory_accesses.all_refs_stored_in_loop.quick_grow
+ memory_accesses.all_refs_stored_in_loop.quick_grow_cleared
(number_of_loops (cfun));
}
--- gcc/cfganal.cc.jj 2023-09-28 11:31:45.013870771 +0200
+++ gcc/cfganal.cc 2023-09-28 12:37:34.302425957 +0200
@@ -468,7 +468,7 @@ control_dependences::control_dependences
bitmap_obstack_initialize (&m_bitmaps);
control_dependence_map.create (last_basic_block_for_fn (cfun));
- control_dependence_map.quick_grow (last_basic_block_for_fn (cfun));
+ control_dependence_map.quick_grow_cleared (last_basic_block_for_fn (cfun));
for (int i = 0; i < last_basic_block_for_fn (cfun); ++i)
bitmap_initialize (&control_dependence_map[i], &m_bitmaps);
for (int i = 0; i < num_edges; ++i)
--- gcc/rtl-ssa/blocks.cc.jj 2023-09-28 11:31:45.413865158 +0200
+++ gcc/rtl-ssa/blocks.cc 2023-09-28 12:41:28.063145949 +0200
@@ -57,7 +57,7 @@ function_info::build_info::build_info (u
// write to an entry before reading from it. But poison the contents
// when checking, just to make sure we don't accidentally use an
// uninitialized value.
- bb_phis.quick_grow (num_bb_indices);
+ bb_phis.quick_grow_cleared (num_bb_indices);
bb_mem_live_out.quick_grow (num_bb_indices);
bb_to_rpo.quick_grow (num_bb_indices);
if (flag_checking)
@@ -614,7 +614,7 @@ function_info::place_phis (build_info &b
// Calculate dominance frontiers.
auto_vec<bitmap_head> frontiers;
- frontiers.safe_grow (num_bb_indices);
+ frontiers.safe_grow_cleared (num_bb_indices);
for (unsigned int i = 0; i < num_bb_indices; ++i)
bitmap_initialize (&frontiers[i], &bitmap_default_obstack);
compute_dominance_frontiers (frontiers.address ());
@@ -626,7 +626,7 @@ function_info::place_phis (build_info &b
// they are live on entry to the corresponding block, but do not need
// phi nodes otherwise.
auto_vec<bitmap_head> unfiltered;
- unfiltered.safe_grow (num_bb_indices);
+ unfiltered.safe_grow_cleared (num_bb_indices);
for (unsigned int i = 0; i < num_bb_indices; ++i)
bitmap_initialize (&unfiltered[i], &bitmap_default_obstack);
--- gcc/tree-ssa-live.cc.jj 2023-09-28 11:31:45.637862015 +0200
+++ gcc/tree-ssa-live.cc 2023-09-28 12:38:25.590706289 +0200
@@ -1361,7 +1361,7 @@ compute_live_vars (struct function *fn,
We then do a mostly classical bitmap liveness algorithm. */
active.create (last_basic_block_for_fn (fn));
- active.quick_grow (last_basic_block_for_fn (fn));
+ active.quick_grow_cleared (last_basic_block_for_fn (fn));
for (int i = 0; i < last_basic_block_for_fn (fn); i++)
bitmap_initialize (&active[i], &bitmap_default_obstack);
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] use *_grow_cleared rather than *_grow on vec<bitmap_head>
2023-09-28 10:47 ` [PATCH] use *_grow_cleared rather than *_grow on vec<bitmap_head> Jakub Jelinek
@ 2023-09-28 11:54 ` Richard Biener
0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2023-09-28 11:54 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Sandiford, gcc-patches
On Thu, 28 Sep 2023, Jakub Jelinek wrote:
> On Thu, Sep 28, 2023 at 12:29:15PM +0200, Jakub Jelinek wrote:
> > On Thu, Sep 28, 2023 at 09:29:31AM +0000, Richard Biener wrote:
> > > > The following patch splits the bitmap_head class into a POD
> > > > struct bitmap_head_pod and bitmap_head derived from it with non-trivial
> > > > default constexpr constructor. Most code should keep using bitmap_head
> > > > as before, bitmap_head_pod is there just for the cases where we want to
> > > > embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
> > > > and in a loop bitmap_initialize it afterwards (to avoid having to
> > > > {quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
> > > > The patch is larger than I hoped, because e.g. some code just used bitmap
> > > > and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
> > > >
> > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > >
> > > OK if there are no comments indicating otherwise.
> >
> > A counter argument against this patch would be that it weakens the intent
> > to catch uses of uninitialized bitmaps for saving a few compile time cycles.
> > If one uses
> > bitmap_head var;
> > bitmap_initialize (&var, NULL);
> > etc., we spend those extra cycles to initialize it and nothing is told that
> > bitmap_initialize overwrites the whole var without ever using of any of its
> > elements, so DSE can't eliminate that. And in the vec case which prompted
> > this patch it was about
> > vec<bitmap_head> a;
> > a.create (n);
> > a.safe_grow (n); // vs. a.safe_grow_cleared (n);
> > for (int i = 0; i < n; ++i)
> > bitmap_initialize (&a[i], NULL);
> > When using bitmap_head_pod, one needs to ensure initialization without
> > help to catch such mistakes.
>
> Here is the alternative patch which pays the small extra price while not
> undermining the checking. Verified in all those places there is a loop
> doing bitmap_initialize immediately afterwards or worst case a few lines
> later.
>
> With the static_assert uncommented, the remaining failures are poly_int
> related (supposedly gone with Richard S.'s poly_int patch) and the
> vect_unpromoted_value/ao_ref still unresolved cases.
OK, I like this better - it's only when we'd sparsely use the vec<>
that it's worth to delay initialization.
Richard.
> 2023-09-28 Jakub Jelinek <jakub@redhat.com>
>
> * tree-ssa-loop-im.cc (tree_ssa_lim_initialize): Use quick_grow_cleared
> instead of quick_grow on vec<bitmap_head> members.
> * cfganal.cc (control_dependences::control_dependences): Likewise.
> * rtl-ssa/blocks.cc (function_info::build_info::build_info): Likewise.
> (function_info::place_phis): Use safe_grow_cleared instead of safe_grow
> on auto_vec<bitmap_head> vars.
> * tree-ssa-live.cc (compute_live_vars): Use quick_grow_cleared instead
> of quick_grow on vec<bitmap_head> var.
>
> --- gcc/tree-ssa-loop-im.cc.jj 2023-09-28 12:06:03.527974171 +0200
> +++ gcc/tree-ssa-loop-im.cc 2023-09-28 12:38:07.028966742 +0200
> @@ -3496,13 +3496,13 @@ tree_ssa_lim_initialize (bool store_moti
> (mem_ref_alloc (NULL, 0, UNANALYZABLE_MEM_ID));
>
> memory_accesses.refs_loaded_in_loop.create (number_of_loops (cfun));
> - memory_accesses.refs_loaded_in_loop.quick_grow (number_of_loops (cfun));
> + memory_accesses.refs_loaded_in_loop.quick_grow_cleared (number_of_loops (cfun));
> memory_accesses.refs_stored_in_loop.create (number_of_loops (cfun));
> - memory_accesses.refs_stored_in_loop.quick_grow (number_of_loops (cfun));
> + memory_accesses.refs_stored_in_loop.quick_grow_cleared (number_of_loops (cfun));
> if (store_motion)
> {
> memory_accesses.all_refs_stored_in_loop.create (number_of_loops (cfun));
> - memory_accesses.all_refs_stored_in_loop.quick_grow
> + memory_accesses.all_refs_stored_in_loop.quick_grow_cleared
> (number_of_loops (cfun));
> }
>
> --- gcc/cfganal.cc.jj 2023-09-28 11:31:45.013870771 +0200
> +++ gcc/cfganal.cc 2023-09-28 12:37:34.302425957 +0200
> @@ -468,7 +468,7 @@ control_dependences::control_dependences
>
> bitmap_obstack_initialize (&m_bitmaps);
> control_dependence_map.create (last_basic_block_for_fn (cfun));
> - control_dependence_map.quick_grow (last_basic_block_for_fn (cfun));
> + control_dependence_map.quick_grow_cleared (last_basic_block_for_fn (cfun));
> for (int i = 0; i < last_basic_block_for_fn (cfun); ++i)
> bitmap_initialize (&control_dependence_map[i], &m_bitmaps);
> for (int i = 0; i < num_edges; ++i)
> --- gcc/rtl-ssa/blocks.cc.jj 2023-09-28 11:31:45.413865158 +0200
> +++ gcc/rtl-ssa/blocks.cc 2023-09-28 12:41:28.063145949 +0200
> @@ -57,7 +57,7 @@ function_info::build_info::build_info (u
> // write to an entry before reading from it. But poison the contents
> // when checking, just to make sure we don't accidentally use an
> // uninitialized value.
> - bb_phis.quick_grow (num_bb_indices);
> + bb_phis.quick_grow_cleared (num_bb_indices);
> bb_mem_live_out.quick_grow (num_bb_indices);
> bb_to_rpo.quick_grow (num_bb_indices);
> if (flag_checking)
> @@ -614,7 +614,7 @@ function_info::place_phis (build_info &b
>
> // Calculate dominance frontiers.
> auto_vec<bitmap_head> frontiers;
> - frontiers.safe_grow (num_bb_indices);
> + frontiers.safe_grow_cleared (num_bb_indices);
> for (unsigned int i = 0; i < num_bb_indices; ++i)
> bitmap_initialize (&frontiers[i], &bitmap_default_obstack);
> compute_dominance_frontiers (frontiers.address ());
> @@ -626,7 +626,7 @@ function_info::place_phis (build_info &b
> // they are live on entry to the corresponding block, but do not need
> // phi nodes otherwise.
> auto_vec<bitmap_head> unfiltered;
> - unfiltered.safe_grow (num_bb_indices);
> + unfiltered.safe_grow_cleared (num_bb_indices);
> for (unsigned int i = 0; i < num_bb_indices; ++i)
> bitmap_initialize (&unfiltered[i], &bitmap_default_obstack);
>
> --- gcc/tree-ssa-live.cc.jj 2023-09-28 11:31:45.637862015 +0200
> +++ gcc/tree-ssa-live.cc 2023-09-28 12:38:25.590706289 +0200
> @@ -1361,7 +1361,7 @@ compute_live_vars (struct function *fn,
> We then do a mostly classical bitmap liveness algorithm. */
>
> active.create (last_basic_block_for_fn (fn));
> - active.quick_grow (last_basic_block_for_fn (fn));
> + active.quick_grow_cleared (last_basic_block_for_fn (fn));
> for (int i = 0; i < last_basic_block_for_fn (fn); i++)
> bitmap_initialize (&active[i], &bitmap_default_obstack);
>
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-28 11:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-28 9:12 [PATCH] bitmap: Introduce bitmap_head_pod Jakub Jelinek
2023-09-28 9:29 ` Richard Biener
2023-09-28 10:29 ` Jakub Jelinek
2023-09-28 10:47 ` [PATCH] use *_grow_cleared rather than *_grow on vec<bitmap_head> Jakub Jelinek
2023-09-28 11:54 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).