This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix DCE REG_LIBCALL note moving from noop move insns (PR rtl-optimization/33644)


On 10/17/07, Eric Botcazou <ebotcazou@libertysurf.fr> wrote:
> > I don't see you, Eric, help with e.g. the mapped-locations support,
>
> Look more carefully: http://gcc.gnu.org/ml/gcc-patches/2007-09/msg01976.html

Sorry I missed that.  I carefully avoid looking over gcc-patches.  I
just have some filters to keep messages with certain buzz-word to sort
out the messages I may find interesting. mapped-locations and Eric are
not buzz-word ;-)

In any case, thanks for having taken care of that..


> > 2) the old DCE in flow.c could remove entire libcall sequences if the
> > libcall result was dead.
> > The new DCE uses REG_LIBCALL_ID notes, so again the reason to have
> > REG_LIBCALL and REG_RETVAL is gone here.
>
> Wrong.  The new DCE handles libcalls backwards, i.e. as optimization barriers
> (see Jakub's messages) instead of as optimization helpers.

Hm, DCE doesn't do what I had expected it to do with REG_LIBCALL_ID
notes.  More on that below...


> > 3) The pre-regalloc scheduling pass used to move libcall blocks as a whole.
> > I don't know if this is still the case, and I also don't really see
> > why this was ever necessary to begin with.  But if this is still
> > necessary for some targets, then you should be able to schedule all
> > insns with the same REG_LIBCALL_ID notes as a block.
>
> These REG_LIBCALL_ID notes would need to be actively maintained throughout the
> entire RTL middle-end

Yes, but that is equally true for REG_LIBCALL and REG_RETVAL, which
currently give GCC a lot of trouble, too.

Since May when I effectively stopped working on gcc, I already
proposed fixes for two libcall notes related bugs (both not in code
coming from the df-branch).  There are two additional problems with
REG_LIBCALL and REG_RETVAL notes. The first is that they must appear
as a pair, which is not intuitive and complicates the notes
maintenance work (one of those two bugs was a case in lower-subreg
where a REG_RETVAL note was removed but not the the corresponding
REG_LIBCALL note. Or the other way around, whatever... :-). The second
is that REG_LIBCALL and REG_RETVAL only implicitly indicate which
insns belong to a libcall sequence, namely all insns appearing between
the libcall and the retval note, inclusive.  ISTR a lot of RTL passes
just punt on libcall sequences to avoid having to figure out whether
an insn is part of a libcall or not.

I thought REG_LIBCALL_ID would have solved these problems, but that
doesn't appear to be the case right now.


> and I don't see any practical advantages over what we
> currently have.

After looking at what dce.c does, I have to agree with you.  I still
see REG_LIBCALL_ID as a _potentially_ better solution, but what dce.c
does now is just as horrible as what other passes have to do with the
"old" libcall notes.

Thoughts dump follows...

Assume REG_LIBCALL and REG_RETVAL wouldn't exist anymore, and GCC
would only have REG_LIBCALL_ID notes.  How would/should things work
then? (Honest open question, so please say what you think about this,
too!)

>From my POV, one advantage of REG_LIBCALL_ID is that all insns that
are part of a libcall sequence are explicitly marked as such.  This
should in theory make it easier for the optimizers to work with the
insns stream, because you can immediately tell whether an insn belongs
to a libcall sequence.  This ought to make it possible to insert
non-libcall insns into the libcall sequence without making it part of
the libcall. That, in turn, would allow the compiler to move around
insns that are part of a libcall. Think e.g. code hoisting or PRE,
when an expression can be hoisted, but one insn is in a libcall and
the other is not -- gcc currently can't hoist in a case like this.
I'm sure similar restrictions due to libcall notes apply most other
RTL passes.  With REG_LIBCALL_ID, most of these restrictions would be
lifted.  Another common problem with the old libcall notes is that you
frequently have to move the notes to another insn in the libcall
sequence.  With REG_LIBCALL_ID you wouldn't have this problem because
all libcall insns carry a libcall note already.


But one thing I don't understand about REG_LIBCALL_ID is how you can
find the result of the libcall sequence.  With the old libcall notes,
when the set of the insn with the REG_RETVAL note is dead, you can
remove the entire libcall sequence (ok, cutting corners a bit here,
but that's more or less what gcc does).  With REG_LIBCALL_ID it is not
quite as obvious when the whole libcall sequence is dead.

So problem #1 is: There is no way AFAICT to tell whether a libcall
sequence is dead if you only have REG_LIBCALL_ID to work with.  One
way around this would be to only remove REG_LIBCALL and to change the
semantics of REG_RETVAL such that it is associated with a libcall ID,
i.e. all insn in a libcall sequence get a REG_LIBCALL_ID note, except
the insn that sets the return value, which gets a REG_RETVAL note
(also holding the libcall ID) instead.


Another thing I don't understand, is how you can find other insns in
the libcall sequence, assuming you would allow inserting non-libcall
insns into the original libcall sequence (and not give them a libcall
note, i.e. as in the case of the patch at the start of the thread I
broke ;-). In dce.c, as I understand it, when an insn with a
REG_LIBCALL_ID is marked as necessary, a linear walk over the insn
chain is done in forward and backward direction over all insn with the
same REG_LIBCALL_ID note value.  This would fail (and apparently does
fail) when a non-libcall insn is inserted into the sequence.  In other
words, passes that use REG_LIBCALL_ID now _already_ assume the libcall
sequence remains contiguous.  This, IMHO, negates the most important
theoretical advantage of REG_LIBCALL_ID over the old libcall notes.

Which is them problem #2: Given a libcall ID, how can you find all
insns belonging to this libcall if the libcall sequence is _not_
contiguous anymore.  One way would be to make the REG_LIBCALL_ID notes
form a double linked list to all other libcall insns.  I think the
overhead of this would be acceptable.  Obviously the trouble would be
to teach the compiler to update the chain.  This is easy when everyone
uses the proper API (i.e. delete_insn and friends) but I'm not sure
this is done consistently everywhere.  Other solutions are possible
(tables, VECs, etc.) but they all have to be maintained.

Of course another solution is to keep the libcall insns sequence
contiguous, but in that case REG_LIBCALL_ID is just overhead to keep
one pass happy.  It would not solve any of the issues with the old
libcall notes.


Yet another solution is to implement TLS_ADDR_EXPR and remove libcall
notes altogether, but that is (and has been for years now)
pie-in-the-sky, it seems :-(

None of this helps solving the current problem for GCC 4.3, sadly...

Gr.
Steven


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]