On 12-10-13 11:37 AM, Steven Bosscher wrote:
On Sat, Oct 13, 2012 at 5:12 PM, Vladimir Makarov wrote:
On 12-10-13 9:40 AM, Steven Bosscher wrote:
Hello,
This fixes the long-standing enhancement request to use DF_LIVE in
IRA. To quote the first comment in the PR:
IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
sets when they are available (typically at O2 and above). The proper
sets can be conveniently accessed using the df_get_live_[in,out]
functions which use DF-LIVE if it is available and fall back to DF-LR
if it is not.
I thought that DF_LIVE wasn't available at -O1 in IRA, but
interestingly ira.c:ira() adds it to the DF-problems list in that case
already:
$ grep -C3 -n df_live_add ira.c
4160-
4161- if (optimize == 1)
4162- {
4163: df_live_add_problem ();
4164- df_live_set_all_dirty ();
4165- }
4166-#ifdef ENABLE_CHECKING
So DF_LIVE is already being computed for IRA. It's just not being used
because the DF_LR_{IN,OUT} macros are used instead of the
df_get_live_{in,out} functions. With my patch, the DF_LIVE sets are
used, which results in marginally faster compile times for my set of
cc1-i files on power64 and on x86_64, as well as smaller code. There's
also a good speedup for the PR54146 test case.
Only reload.c still uses the DF_LR_OUT. The comment in
find_dummy_reload() suggests this is intentional.
I also fond a bug in IRA:
4392- touch the artificial uses and defs. */
4393- df_finish_pass (true);
4394- if (optimize > 1)
4395: df_live_add_problem ();
4396- df_scan_alloc (NULL);
4397- df_scan_blocks ();
4398-
At optimize>1 the DF_LIVE problem is already always computed. I think
the idea here was to *remove* the DF_LIVE problem at -O1, which is
also part of the attached patch.
(Perhaps we should even simply not add the DF_LIVE problem at -O1, but
that can be done in a follow-up patch if necessary.)
Bootstrapped and tested (with default checking and with release
checking) on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
OK for trunk?
LIVE is not used on purpose. I added LIVE usage to the old RA about 10
years ago (it was before DF-infrastructure). Everything was ok until,
somebody reported compiler crash on semantically wrong program (something
with usage of uninitialized variable). After that I removed this code.
GCC was a completely different compiler 10 years ago. Handling of
uninitialized variables is radically different since tree-ssa, and
also the resource allocation side of the compiler has essentially been
rewritten (by you :-). Whatever broke in the compiler 10 years ago may
not be relevant anymore today.
I can't find your work, or the reported problem, in the gcc mailing
list archives. Did you add a test case for the problem, and if so,
where can I find it?
Probably, that is also a reason why reload uses LR not LIVE.
The reason why reload uses DF_LR is PR20973, as documented in the code.
(And FWIW, that bug doesn't reproduce even with GCC 4.0 if you add the
transformations of pass_initialize_regs to it, that pass exists to
avoid uninitialized regs hazards by making them initialized.)
So I'd like to
wait a few weeks. When I have more time, I am going to reproduce this test
suite and look how IRA behaves on it.
I don't think that's giving this patch a fair chance. In a few week's
time, stage1 will be over. You may not have time, but that doesn't
mean other developers should wait. If you have a test case that fails
with my patch, I'll gladly spend my own time on it to see if it can be
fixed.
My patch was properly tested on two primary targets and passes all
tests in the test suite, it improves the compiler now, and it
addresses a PR that you've basically ignored for 4 years. I kindly ask
you to reconsider your position :-)
I guess you are right. A lot of things are changed. DF is also
different from what I used. Other passes also uses LIVE sets.
Ok for the idea. If we have a problem later, we could fix it. I'll
look at the next version of the patch when you send it to give your the
final approval.
Thanks for your attention to IRA.
By the way, I found the reported problem:
http://gcc.gnu.org/ml/gcc-patches/2004-06/msg00533.html
But it occurred on s390.
