This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: Limits of stage3 changes
On Nov 18, 2007 10:29 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> I think the answer is that the patch is not a priori unacceptable.
>
> But, given that we're talking about a relatively large change, I think
> the bar should be set higher than for a change to just a few lines of
> code. In particular, I would want to see that there is, in fact,
> measurable improvement to compile-time -- whereas in Stage 1, we might
> just approve the patch on the grounds that it moves us towards the
> infrastructure that we want to be using throughout the compiler.
OK, that was more or less what I was expecting.
I don't expect the compile time impact to be very significant. The
changes in the patch I posted would remove 2 passes over all RTL insns
when compiling with -fgcse. That is significant but not
earth-shattering, compared to the ~80 per function we do during a
compilation. Plus, the real bottle-necks in gcse.c are not in
compute_sets() and in the computation of CUIDs, but in the dataflow
solver and functions like find_avail_set().
(Those bottle-necks are also fixable (they are gone in my CPROP
rewrite) but that's something I would not even want to contribute for
stage3 ;-))
> I would also want a relevant maintainer to carefully review the patch.
> It might be that even though we think the patch is likely to be correct,
> either that maintainer, or I, decide that there's too much associated
> risk.
Not to brag, but I think I know df and gcse as well as anyone. So I
know the associated risks, and I can share my thoughts about them with
you ;-)
My biggest worry was that the removal of CUIDs would break something,
and the patch as posted indeed does break PRE (causes ICEs when taking
the INSN_CUID of a deleted insn). The removal of compute_sets() is
quite safe, because compute_sets() computes exactly the same
information as df-scan does.
> So, I don't want to promise that we would accept the patch in
> Stage 3, either.
OK, then, all things considered, I think I can better spend my time on
something else than this patch.
> Steven, I recognize that might not be as definitive an answer as you
> would like, but I hope you will understand my thinking.
No, this is the kind of answer I was looking for. I fully understand
your thinking, and I just wanted to check my own thoughts against the
opinions on the list. So thanks for the elaborate answer!
Gr.
Steven