On Nov 18, 2007 10:29 PM, Mark Mitchell <[EMAIL PROTECTED]> 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
