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 :-)
Ciao!
Steven
