Re: no_new_pseudos

[Alexandre Oliva <aoliva at redhat dot com>]

On Jul  9, 2007, Ian Lance Taylor <iant@google.com> wrote:

> Alexandre Oliva <aoliva@redhat.com> writes:
>> On Jul  8, 2007, Ian Lance Taylor <iant@google.com> 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   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}