On 12/16/2016 06:57 AM, Richard Biener wrote:
Apart from what Trevor says about using sbitmaps (try to avoid the initial
zeroing please) and the missed freeing (you can use auto_[s]bitmap?)
some comments below.
New version uses sbitmaps and avoids zero-ing when we can.
+static void
+trim_complex_store (bitmap orig, bitmap live, gimple *stmt)
+{
+ bitmap dead = BITMAP_ALLOC (NULL);
+ bitmap_and_compl (dead, orig, live);
+
+ /* So we have to verify that either the real or imag part as a whole
+ is dead. They are always the same size. Thus for one to be dead
+ the number of live bytes would have to be the same as the number of
+ dead bytes. */
+ if (bitmap_count_bits (live) == bitmap_count_bits (dead))
popcount is expensive, so is this really a short-cut?
Note this can all be done more efficiently. Using sbitmaps forces us to
normalize the offset/size and I've dropped the original bitmap as the
ao_ref carries the data we need. Those two changes make other
simplifications fall-out and what we get in compute_trims a last_set_bit
and first_set_bit on the live bitmap -- no other bitmap
traversals/searches/counts.
That makes compute_trims more efficient than the code in
trim_complex_store. So I've made trim_complex_store re-use the more
general trimming code, and it's still agnostic of the underlying sizes,
it just cares that the underlying components are the same size.
The actual transform in dse_possible_dead_store_p looks a bit misplaced.
I see it's somehow convenient but then maybe returning a enum from this
function might be cleaner. Well, I'm not too torn about this, so maybe
just rename the function a bit (no good suggestion either).
The rest of the patch (the infrastructure) looks reasonable.
I think I mentioned it earlier, but moving towards a single allocated
bitmap for the entire pass makes it much easier to have
dse_possible_dead_store_p to return a tri-state and live information.
You'll see that change in the upcoming revision to the patchkit.
Jeff
