This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: IRA plans and latest version of IRA patch
Vladimir Makarov wrote:
Richard Guenther wrote:
The plan works for me, but I think you need approval for the target
and middle-end parts that are not part of "register allocation" which
you are maintainer of. The easiest would be probably if some GWP
acks the patch. The reload and caller-save parts stick out, but I
didn't try to classify if they are register allocator or not.
All the mechanical parts (timevar stuff, passes.c, etc.) are ok.
Thanks, Richard. Then I'll wait for approval of the target parts.
They are pretty simple. It is just adding IRA_COVER_CLASSES macros.
Many of them I've got from the target maintainers. All target changes
different from IRA_COVER_CLASS I got from the maintainers.
I'd say that caller-save is a part of the reload. But I am not sure
too that the reload (taking its complexity) is a part of the register
allocator. Although I believe I know the reload pretty well, Bernd
for sure knows it better. And it would be nice him to look at that
changes and give me an approval.
Bernd, here is the brief description of the reload changes:
I've started looking at the patch today. A real review would take
several full-time weeks, which I'm afraid I don't have, but I'll try to
do something useful. Some initial comments below.
As far as reload is concerned, one of the most important things to
ensure is that we must keep the guarantee that it terminates. Both
pseudo_previous_regs and used_spill_regs are currently treated in a way
that should guarantee termination, but these safeguards are disabled by
the IRA patch with no obvious replacement, which is worrying at first
glance.
o Finding reloads for most frequently executed insns first. Sorting
insn chain is done for this by calling sort_insn_chain in
reload1.c::reload. By processing most frequently executed insns
first, we have a chance to provide them better hard registers for
the reloads.
This sounds like something that could possibly go in independently?
Anything we can break off the huge monolithic chunk makes life easier
for reviewers and testers. Even if it's more work for you - please
consider that it'll most likely lead to a better end result.
o Better choosing spill register. The reload call IRA
better_spill_reload_regno_p in reload1.c:find_reg to find pseudo to
spill for reloads. Besides old heuristic based on spill costs, IRA
uses secondary heuristic based on length of the pseudo live range
where the register pressure is too high.
Likewise - is this something that can be done independent of which
register allocator is used?
o Using hard-registers which saved around calls for the reload
registers.
- A new member saved is added to structure chain. It is used for
communication between caller-save and the reload. Caller-save
adds a pseudo which saved at given point to the bitmap of the
corresponding insn chain, the reload (in find_reg) uses this info
to find a hard register for given insn reloads.
I'll need a lot more time to digest the caller-save changes. At what
times does the new caller-save code allocate pseudos?
o Sharing stack slots used for saving restoring hard-registers. It is
done mostly in setup_save_areas. We build callee-clobbered hard
register conflicts, and then allocate stack slots for them most
frequently used first.
See above - is this a change that can be made independently?
The dataflow equations are a bit complicated
They are also under-documented. A lot of functions in caller-save.c
have useless comments of the form
+/* The function used by the DF equation solver to propagate save info
+ from successor to predecessor on edge E. */
Please document more clearly what you are computing, and why.
There are many other places where more documentation is desirable. To
give just a single example, what does the save_pseudo array in
calculate_local_save_info contain?
I still think the patch is exposing too many new options. Should users
really have to (want to) decide whether to use -fno-ira-move-spills?
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif