Re: [PATCH] splits after reload

[Joern Rennecke <joern dot rennecke at superh dot com>]

> To avoid compile time overhead for other targets, a target hook
> looked reasonable. Other targets can make use of that hook
> (if some are facing similar problems) and solve their "target specific
> problem in a target specific way".

I don't think the objection was against a target hook in general, but
against putting it into the splitting machinery and making it do something 
that is not actually splitting.

> Running a pass over all insns generated by the splitter looks expensive.

No, not necessarily.  When you do a full pass over all insns you can
keep track of some information in a way that is impossible if you
have to look ad-hoc at a lot of individual instructions.

Also, instead of scanning ahead from individual instructions, you can
just remember what you might want to fix up and keep on scanning in
a single sweep.  Thus, while the hook solution os O(n*n) for the
size of basic blocks, the single-pass can be done in something
similar to O(n*log(n))

> It sounds like if SH needs some optimization it has to pay "tax"
> going through all the condition checks which are bound to be
> the part of generic optimizer. 

If the SH does some optimization on its own, you have to pay "tax"
maintaining it separately as the compiler code evolves.
The cost of doing something in a general way is usually just a linear factor,
while algorithmic improvements can be dramatic, and they tend to require
tighter coupling of data structures to the rest of the compiler than
is easily maintainable in a target port.

So, if you can do the optimization in a machine-independent, generally
usable way, go for it.

Moreover, even in cases where you think that your approach is a little
simpler than the one that the reviewer suggests, if the alternate approach
is workable, and gives you the same code quality, it is more constructive
just to implement the alternate approach than to drag on the discussion.
Consider how many people read this list.
If you come against problems with an alternate approach, we can discuss
these, and maybe everyone learns something from the effort.

> reload_cse is not able to perform this optimization, even if
> you run it after flow2. There is no code written in reload_cse for handling
> such problems. Please correct me if I'm wrong.

reload_combine and reload_cse_move2add are O(n) subpasses of
reload_cse_regs.  It seems your optimizations can be added to either
of them with little effort.

reload_combine scans backwards over instructions, keeping track of how
registers are being used.
reload_cse_move2add scans forward over instructions, keeping track of
some values in registers, i.e. constants and sums of registers and
constants.

> If reload_cse () ought to do so, 
> I feel that needs rewriting of reload_cse which certainly is not a
> trivial task. The target hook should be acceptable in the meantime.

I think we are talking about about a screenful of code here.
The main work, of course, is the testing and benchmarking.

> I agree that recombination is not the purpose of split_all_insns ().
> But a target hook helps without harming any other target.
> Please have a look at the patch and let me know what is wrong with it.

Most design decisions that involve style or cleanliness are right or wrong
only when you make the design principles explicit, which would be a lot of
work for gcc.
What we can objectively say is that the hook solution is slower on large
basic blocks than an implementation in reload_combine or reload_cse_move2add,
and the hook solution also leads to more code that is harder to maintain.