On 01/04/2017 06:22 AM, Richard Biener wrote:
Bootstrapped and regression tested on x86_64-linux-gnu. OK for the trunk?
New functions in sbitmap.c lack function comments.
Bah. Sophomoric on my part. Fixed.
bitmap_count_bits fails to guard against GCC_VERSION >= 3400 (the version
is not important, but non-GCC host compilers are). See bitmap.c for a
fallback.
Mistake on my part. I keep thinking we support starting the bootstrap
process with the most recently released GCC, but we support 3.4 as well
as other C++98/C++03 compilers. Fixed in the next update (and tested by
forcing the fallback method).
Both bitmap_clear_range and bitmap_set_range look rather inefficient...
(it's not likely anybody will clean this up after you)
They were, but not anymore. Now they build a mask to deal with any
partial clearing/setting in the first word, then a single memset for any
whole words in the middle, then another masking operation on residuals
in the last word. Verified behavior by by keeping two bitmaps, one with
the old slow approach and one with the faster implementation and
checking for equality. Obviously those verification bits won't be in
the final patch.
I'd say split out the sbitmap.[ch] changes.
Sure. That's easy enough.
+DEFPARAM(PARAM_DSE_MAX_OBJECT_SIZE,
+ "dse-max-object-size",
+ "Maximum size (in bytes) of objects tracked by dead store
elimination.",
+ 256, 0, 0)
the docs suggest that DSE doesn't handle larger stores but it does (just in
the original limited way). Maybe "tracked bytewise" is better.
Agreed and fixed.
+static bool
+valid_ao_ref_for_dse (ao_ref *ref)
+{
+ return (ao_ref_base (ref)
+ && ref->max_size != -1
+ && (ref->offset % BITS_PER_UNIT) == 0
+ && (ref->size % BITS_PER_UNIT) == 0
+ && (ref->size / BITS_PER_UNIT) > 0);
I think the last test is better written as ref->size != -1.
Seems reasonable. Fixed.
Btw, seeing you discount non-byte size/offset stores this somehow asks
for store-merging being done before the last DSE (it currently runs after).
Sth to keep in mind for GCC 8.
Yea, probably. Of course it may also be the case that DSE enables store
merging. Worth some experimentation.
+/* Delete a dead store STMT, which is mem* call of some kind. */
call STMT
Fixed.
+static void
+delete_dead_call (gimple *stmt)
+{
+
excess vertical space
Likewise.
......
+ if (lhs)
+ {
+ tree ptr = gimple_call_arg (stmt, 0);
+ gimple *new_stmt = gimple_build_assign (lhs, ptr);
+ unlink_stmt_vdef (stmt);
+ if (gsi_replace (&gsi, new_stmt, true))
+ bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index);
release_ssa_name (gimple_vdef (stmt));
+ { m_live_bytes = sbitmap_alloc (PARAM_VALUE
(PARAM_DSE_MAX_OBJECT_SIZE));m_byte_tracking_enabled = false; }
formatting.
Yea. Fixed.
The DSE parts look good to me with the nits above fixed. Just re-spin
the sbitmap.[ch] part please.
Will repost the sbitmap.c bits after retesting the series.
jeff
