| Message ID | 4C18F4F4.606@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
> This patch adds several open-ended comments in gcse.c. I'll be happy > if anyone can answer some of them, in which case I'll check in the > answers, rather than questions :). > > Thank you, > @@ -3431,7 +3431,9 @@ process_insert_insn (struct expr *expr) For PRE, we want to verify that the expr is either transparent or locally anticipatable in the target block. This check makes - no sense for code hoisting. */ + no sense for code hoisting. + ??? We always call this function with (PRE == 0), which makes the checks + useless. */ See pre_edge_insert and search for EDGE_ABNORMAL. That code went through several iterations and may no longer be necessary. So we really should extend the existing comment before the call to insert_insn_end_basic_block from pre_edge_insert. @@ -3535,6 +3537,9 @@ insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) else new_insn = emit_insn_after_noloc (pat, insn, bb); + /* ??? It maybe useful to try set REG_EQUAL note on NEW_INSN here. + How can we do it? */ + Why do you think this is important? Have you run into cases where having the note showing an alternate form of the expression would have allowed further optimization? So, when you find a hoistable expression that reaches from its new block to a dominated child block which also evaluates the expression, if the insn in the dominated child has a REG_EQUAL note, you might be able to copy it. So record it into a variable in the loop over the dominated blocks. At the end of that loop, copy the REG_EQUAL note to the new insn (which you'll need to record as well). You may have to verify the note is safe to copy/move. I haven't pondered that aspect at all. /* Walk over each basic block looking for potentially hoistable - expressions, nothing gets hoisted from the entry block. */ + expressions, nothing gets hoisted from the entry block. + + ??? It maybe worthwhile to walk CFG in DFS order over the dominator tree. + One can imagine a case when a dominated block B is linked before + its dominator A, so if expressions were hoisted from blocks C and D, + which B (and A) dominates, then it may occur that we miss + an optimization of moving these expressions all the way to A. + Alternatively, we may handle this case by updating expressions' + occurences to include instructions emitted by code hoisting, i.e., + an expression emitted at the end of B will then be hoisted to A. */ Well, yea, I guess this is possible. I'm not sure if it happens much in practice. I think your best bet would be controlling the order of blocks visited. Trying to update the tables on the fly seems like it's going to get ugly quick. Jeff
diff --git a/gcc/gcse.c b/gcc/gcse.c index c81d71c..7215063 100644 --- a/gcc/gcse.c +++ b/gcc/gcse.c @@ -3431,7 +3431,9 @@ process_insert_insn (struct expr *expr) For PRE, we want to verify that the expr is either transparent or locally anticipatable in the target block. This check makes - no sense for code hoisting. */ + no sense for code hoisting. + ??? We always call this function with (PRE == 0), which makes the checks + useless. */ static void insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) @@ -3535,6 +3537,9 @@ insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) else new_insn = emit_insn_after_noloc (pat, insn, bb); + /* ??? It maybe useful to try set REG_EQUAL note on NEW_INSN here. + How can we do it? */ + while (1) { if (INSN_P (pat)) @@ -4343,7 +4348,16 @@ hoist_code (void) index_map[expr->bitmap_index] = expr; /* Walk over each basic block looking for potentially hoistable - expressions, nothing gets hoisted from the entry block. */ + expressions, nothing gets hoisted from the entry block. + + ??? It maybe worthwhile to walk CFG in DFS order over the dominator tree. + One can imagine a case when a dominated block B is linked before + its dominator A, so if expressions were hoisted from blocks C and D, + which B (and A) dominates, then it may occur that we miss + an optimization of moving these expressions all the way to A. + Alternatively, we may handle this case by updating expressions' + occurences to include instructions emitted by code hoisting, i.e., + an expression emitted at the end of B will then be hoisted to A. */ FOR_EACH_BB (bb) { int found = 0;