I still have misgivings about all the changes needed to the following
passes, but I guess there's no choice but to live with it. So, I'm
trying to look at this patch, but I'm finding it fairly impenetrable and
underdocumented.
+ /* The concerns for which we want a prologue placed on this BB. */
+ sbitmap pro_concern;
+
+ /* The concerns for which we placed code at the start of the BB. */
+ sbitmap head_concern;
What's the difference?
+ /* The frequency of executing the prologue for this BB and all BBs
+ dominated by it. */
+ gcov_type cost;
Is this frequency consideration the only thing that attempts to prevent
placing prologue insns into loops?
+
+/* Destroy the pass-specific data. */
+static void
+fini_separate_shrink_wrap (void)
+{
+ basic_block bb;
+ FOR_ALL_BB_FN (bb, cfun)
+ if (bb->aux)
+ {
+ sbitmap_free (SW (bb)->has_concern);
+ sbitmap_free (SW (bb)->pro_concern);
+ sbitmap_free (SW (bb)->head_concern);
+ sbitmap_free (SW (bb)->tail_concern);
+ free (bb->aux);
+ bb->aux = 0;
+ }
+}
Almost makes me want to ask for an sbitmap variant allocated on obstacks.
+ /* If this block does have the concern itself, or it is cheaper to
+ put the prologue here than in all the descendants that need it,
+ mark it so. If it is the same cost, put it here if there is no
+ block reachable from this block that does not need the prologue.
+ The actual test is a bit more stringent but catches most cases. */
There's some oddness here with the leading whitespace.
+/* Mark HAS_CONCERN for every block dominated by at least one block with
+ PRO_CONCERN set, starting at HEAD. */
I see a lot of code dealing with the placement of prologue
parts/concerns/components, but very little dealing with how to place
epilogues, leading me to wonder whether we could do better wrt the
latter. Shouldn't there be some mirror symmetry, i.e.
spread_concerns_backwards?
+ {
+ if (first_visit)
+ {
+ bitmap_ior (SW (bb)->has_concern, SW (bb)->pro_concern, concern);
+
+ if (first_dom_son (CDI_DOMINATORS, bb))
+ {
+ concern = SW (bb)->has_concern;
+ bb = first_dom_son (CDI_DOMINATORS, bb);
+ continue;
+ }
Calling first_dom_son twice with the same args? More importantly, this
first_visit business seems very confusing. I'd try to find a way to
merge this if with the places that set first_visit to true. Also -
instead of having a "continue;" here it seems the code inside the if
represents an inner loop that should be written explicitly. There are
two loops with such a structure.
+/* If we cannot handle placing some concern's prologues or epilogues where
+ we decided we should place them, unmark that concern in CONCERNS so
+ that it is not wrapped separately. */
+static void
+disqualify_problematic_concerns (sbitmap concerns)
+{
+ sbitmap pro = sbitmap_alloc (SBITMAP_SIZE (concerns));
+ sbitmap epi = sbitmap_alloc (SBITMAP_SIZE (concerns));
+
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, cfun)
+ {
+ edge e;
+ edge_iterator ei;
+ FOR_EACH_EDGE (e, ei, bb->succs)
+ {
+ bitmap_and_compl (epi, SW (e->src)->has_concern,
+ SW (e->dest)->has_concern);
+ bitmap_and_compl (pro, SW (e->dest)->has_concern,
+ SW (e->src)->has_concern);
What is the purpose of this?
+/* Place code for prologues and epilogues for CONCERNS where we can put
+ that code at the start of basic blocks. */
+static void
+do_common_heads_for_concerns (sbitmap concerns)
The function name should probably have some combination of the strings
emit_ and _at or _into to make it clear what it's doing. This and the
following function have some logical operations on the bitmaps which are
not explained anywhere. In general a much better overview of the
intended operation of this pass is needed.
+ {
+ bitmap_and_compl (epi, SW (e->src)->has_concern,
+ SW (e->dest)->has_concern);
+ bitmap_and_compl (pro, SW (e->dest)->has_concern,
+ SW (e->src)->has_concern);
+ bitmap_and (epi, epi, concerns);
+ bitmap_and (pro, pro, concerns);
+ bitmap_and_compl (epi, epi, SW (e->dest)->head_concern);
+ bitmap_and_compl (pro, pro, SW (e->dest)->head_concern);
+ bitmap_and_compl (epi, epi, SW (e->src)->tail_concern);
+ bitmap_and_compl (pro, pro, SW (e->src)->tail_concern);
Likewise here.
Bernd
