This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [SH] PR 53976 - Add RTL pass to eliminate clrt, sett insns

On Wed, Nov 20, 2013 at 8:29 PM, Oleg Endo wrote:
> The attached patch adds another SH specific RTL pass which is supposed
> to optimize clrt and sett instruction usage.  As a start, it currently
> just eliminates redundant clrt and sett instructions in cases where the
> T bit value is known.  However, I'm planning on extending it a little in
> order to e.g. hoist clrt/sett insns out of loops etc.

+#include <vector>

> +#define log_msg(...)\
> +  do { if (dump_file != NULL) fprintf (dump_file, __VA_ARGS__); } while (0)

Is that valid C++98, a varags macro?

> +    // Notice that the CFG might be invalid at late RTL stages and
> +    // BLOCK_FOR_INSN might return null.  Thus the basic block are recorded
> +    // here while traversing them.
> +    basic_block bb;

You insert your pass just before sched2. The CFG is perfectly fine at
that stage. So you shouldn't need this. (And if you would need to
record bb, then this solution wouldn't be GC-safe).

BLOCK_FOR_INSN will only be NULL for things that are not inside a
basic block (some notes, deleted labels, etc.).

That all said and done: AFAICT you don't actually use BLOCK_FOR_INSN
anywhere :-)

> +    for (edge_iterator ei = ei_start (bb->preds); !ei_end_p (ei); ei_next (&ei))


Declaring the edge_iterator inside the for() is not a good argument
against FOR_EACH_EDGE. Of course, brownie points are up for grabs for
the brave soul daring enough to make edge iterators be proper C++
iterators... ;-)

> +    if (pred_bb->index == ENTRY_BLOCK)

I used to dislike this idiom of checking bb->index against fixed block
numbers. But now that ENTRY_BLOCK_PTR and EXIT_BLOCK_PTR actually
require two pointer dereferences (cfun->cfg->...) it's the better

/me adds to TODO list...

> +::remove_ccreg_dead_unused_notes (std::vector<ccreg_value>& values) const

Maybe put a generic version of this in rtlanal.c, next to

On the whole: The pass will have worst-case run time quadratic in the
number of basic blocks, due to the recursion in
find_last_ccreg_values. You'll probably want to limit the depth of the
iteration somehow (hard-coded depth, common dominator, first
post-dominated block, whatever...) or sooner-or-later you'll have a PR
assigned to you for a test case that makes compile time explode in
this pass...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]