On Jul 9, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote: > Alexandre Oliva <[EMAIL PROTECTED]> writes: >> On Jul 8, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote: >> >> > But since these aspects of the register allocator are not at all >> > likely to change, wouldn't it be a waste of time to prepare for them >> > now? >> >> Yup. But from that to concluding that we should remove the clear >> abstraction that enables someone to prepare for them right now, that >> is useful, mnemonic, clear in meaning, and currently functional. >> >> Replacing that with some variable that denotes some internal state in >> the middle end and requiring the back end to use it is exposing the >> guts of the middle end to the back end. That's breaking abstraction >> layers. That's bad software engineering in general.
> Note that that is exactly what was happening before. no_new_pseudos > denoted internal state in the middle end. In a sense, you're right, and I'd never perceived it that way. Maybe it's because I've always perceived it as something turned towards a users' needs (can I create new pseudos?) rather than as something turned towards internal implementation details (do I need to resize register tables afterwards?) That it served both purposes might or might not be a sign of the bad practice of abstraction overloading. Honestly, I was never even aware of the possibility of resizing the tables once no_new_pseudos was set, or even that such tables existed or had anything to do with this flag. To me, no_new_pseudos was part of the gen_reg_rtx API, used to tell its callers that they needed to work around their hunger for pseudos. This is what I get from gccint: The global variable `no_new_pseudos' can be used to determine if it is unsafe to create new pseudo registers. If this variable is nonzero, then it is unsafe to call `gen_reg_rtx' to allocate a new pseudo. I realize now that the comment before the no_new_pseudos declaration in rtl.h mentions life analysis as the point in which no_new_pseudos is set. This does support the claim that it was there for a different purpose. However, the fact that it can be safely replaced by (reload_in_progress || reload_completed) shows the strength of the abstraction it represents. We could change the internal implementation of the compiler in such a major way as to enable pseudos to be created much later in the game (even if this option is not actually exercised) while having to adjust as simple a point as when/how to set no_new_pseudos. Now, it is true that the overspecification of the comment above its declaration may have led people to use it for different purposes. While some may have used it to test whether new pseudos can be created, others may have used it to test whether we're on or past life analysis. Whether these will break as we relax the restrictions on creation of new pseudos is something that only auditing and testing will tell. This is a negative side effect of abstraction overloading. > But I believe that repurposing is bad because the backends do not and > should not use no_new_pseudos consistently. Consistent usage would > require them to check it far more often than they currently do. That > would be counterproductive and even impossible. Instead, the backends > only check it for the specific routines which may be called during > register allocation. Actually, this is all part of the contract between the middle end and the back end. Even if it's implicit, those "in the know" understand that only a few expanders (movM, addM) and splitters are ever supposed to be called with no_new_pseudos set. That's where it needs to be tested if gen_reg_rtx() needs to be called. Does this mean all of them do? Maybe not. Maybe some are failing to do their part in the API contract and they have never been caught in error. Does this mean they should? Yes, certainly. Does this mean we should extend the contract to cover all expanders? Only if there were good reasons to do so. > Since those are the only backend routines which must check > no_new_pseudos, and those are the only backend routines which do check > no_new_pseudos, I think that trying to reclaim no_new_pseudos as a > generic concept separate from register allocation is inappropriate. Looks like it's never been directly related with register allocation, ever since its introduction in revision 23855, in Nov 25, 1998. But since then it's always been checked by gen_reg_rtx, so its use as a test on whether gen_reg_rtx() can be safely called has historically been supported. Can the same be said of any other suggested meaning for this abstraction? > Since we have no reason to believe that the backend should not > create new pseudo-registers before register allocation, How about during local or global? These are part of register allocation, they're not covered by the macro you proposed, but one might assume they are by looking at the macro name. At which point, which meaning would be right? > Perhaps a root of the disagreement is that I believe that unnecessary > abstraction is actually harmful. We are not writing a general purpose > library or API here, for either the middle-end or the backend. We are > writing a tightly coupled single program. I don't think it's useful to think of it as so tightly coupled. That Kenny is not comfortable modifying all backends shows that it's in our best interest to keep it loosely coupled. To retain stable, well-defined interfaces between the layers as much as possible, such that people can modify the layers they're familiar with without too much worries about having to make global changes. And it's not like we have to get out of our way to start introducing new abstractions to reduce coupling, but removing abstractions that do so should take it into account. Very few active global maintainers feel comfortable tweaking anything anywhere in the compiler these days. It's about time we start using good software engineering practices to overcome the lack of all-knowing people. GCC has grown a lot, and it's not going to stop growing any time soon. >> > For these reasons, since the underlying meaning of no_new_pseudos is >> > no longer necessary, >> As in, we can now create new pseudos at any time throughout the >> compilation and expect them to be handled correctly? I don't think >> so. > Since that would be obviously nonsensical, it must not be what I > meant. I must have meant what I said: "it was there to remind people > to call the appropriate functions to resize the register arrays." I don't see how this is implied in the no_new_pseudos name, or documented where no_new_pseudos is documented. This means to me that the resizing of arrays is an internal implementation detail that shouldn't be of concern of users of this abstraction. The fact that I've used it correctly for so many years without having ever been aware of these register arrays shows how successful the abstraction was. > I believe that you are restating the underlying meaning of > no_new_pseudos from what it originally was. The available documentation appears to support the meaning I've had for it, and in one of the three documentation spots, another meaning (that we're past life analysis), none of which is the one you claim it to be. Which is not to say that the internal implementation doesn't give it other meanings that are useful, or show why it was necessary or useful to define it the way it was defined. It's just that the stated purpose, the documented API, the contract of use, all point in the direction in which I've always understood and used it. Unless you can show documentation that states other intended uses for it, I stand by my claim that this is what it means. -- 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}