On Jul 6, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote: > Richard Sandiford <[EMAIL PROTECTED]> writes: >> That's why it seems so odd to me to want to get rid of the port uses >> and not replace it with something directly equivalent. I just don't >> see how it qualifies as a clean-up. I think tying the ports even >> more to reload-specific conditions, even when we already have a more >> abstract concept, is the wrong way to go.
> At the risk of disturbing the bikeshed painting, what do you think of > this patch? It's better, because it retains an easy-to-search-for interface, but it still doesn't match the abstraction expected by back-end uses. Let me try to explain why it is important to retain this interface. First, a trivial nit: regalloc_started_p should become true when local(-regalloc) started, and I assume you meant it to become true only when reload started. This might seem like an irrelevant point, but I'll show below why it is more relevant than it seems. regalloc_started_p is still at a lower level of abstraction than no_new_pseudos. Just because no_new_pseudos can be currently implemented in terms of regalloc_started_p, it's not obvious that it will remain so. Consider changes in reload that enable it to cope with the creation of new pseudos up to a certain point. Would we then have to change all occurrences of regalloc_started_p() that mean no_new_pseudos to (regalloc_started_p() && !new_pseudos_still_ok_p ()). Consider that we drop reload entirely and come up with a global allocator that can cope with new pseudos up to a certain point, but not all the way to the end. Do we then change regalloc_started_p() such that it becomes true only in the middle of this global allocator? Or audit all uses of regalloc_started_p() so as to verify wether it means no_new_pseudos or actually regalloc_started_p? Consider that local and or global are changed in such a way that they start using expanders that currently use no_new_pseudos. Well, surely expanders that create new pseudos after local or global get poorer register allocation than those that run before register allocation starts. Someone looking at the backends might think regalloc_started_p() is there for performance reasons, rather than for correctness reasons, and make incorrect decisions based on that. At some point, it may be difficult to tell whether certain uses were mis-uses based on an incorrect understanding of what regalloc_started_p really stood for, because it could arguably mean both. Some might even be tempted to change its definition such that it is "more correct", and then ports would break that depended on the semantics of no_new_pseudos to work. Arguably, we should have another abstraction to enable ports to tell not only whether it's *possible* to create new pseudos, but whether it's *efficient* to do so. This pair of abstractions could then be implemented in terms of a similar enumeration to the one I suggested before, but adding yet another value: enum { BEFORE_REGALLOC = -2, DURING_REGALLOC = -1, DURING_RELOAD = 0, AFTER_RELOAD = 1 } regalloc_state; #define reload_in_progress (regalloc_state == DURING_RELOAD) #define reload_completed (regalloc_state > DURING_RELOAD) #define no_new_pseudos (regalloc_state >= DURING_RELOAD) #define inefficient_new_pseudos (regalloc_state > BEFORE_REGALLOC) And then, had we introduced this abstraction you proposed, we might end up with something like: #define regalloc_started_p() (regalloc_state >= DURING_RELOAD) #define real_regalloc_started_p() (regalloc_state >= DURING_REGALLOC) See why imprecise abstractions are a problem, and why lowering abstractions just because it's possible ATM, without any performance or maintainability gains to justify them, is a losing proposition in the long run? -- Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/ FSF Latin America Board Member http://www.fsfla.org/ Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org} Free Software Evangelist [EMAIL PROTECTED], gnu.org}