RFA: patch for register pressure sensitive insn scheduling

[Richard Guenther]

On Thu, Sep 3, 2009 at 4:48 PM, Vladimir Makarov<vmakarov@redhat.com> wrote:
> Richard Guenther wrote:
>>
>> On Thu, Sep 3, 2009 at 4:30 PM, Vladimir Makarov<vmakarov@redhat.com>
>> wrote:
>>
>>>
>>> Richard Guenther wrote:
>>>
>>>>
>>>> On Wed, Sep 2, 2009 at 5:41 PM, Vladimir Makarov<vmakarov@redhat.com>
>>>> wrote:
>>>>
>>>>
>>>>>
>>>>> Vladimir Makarov wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> David Edelsohn wrote:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Vlad,
>>>>>>>
>>>>>>> Thanks for investigating and addressing this issue in GCC.
>>>>>>>
>>>>>>> Did you test on POWER6 with the rs6000.c:rs6000_issue_rate()
>>>>>>> restriction for the first scheduling pass removed?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> I used the same issue rate (1 for the 1st insn scheduling) as it is
>>>>>> defined currently in the trunk.
>>>>>>
>>>>>> I remembered the discussion about this restriction a few weeks ago.  I
>>>>>> should try to check SPEC without the restriction.
>>>>>>
>>>>>> So I'll do it and post the results (probably tomorrow).
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> I've checked register-pressure scheduling (using -fsched-pressure) with
>>>>> and
>>>>> without the constraint in rs6000_issue_rate:
>>>>>
>>>>>  if (!reload_completed)
>>>>>  return 1;
>>>>>
>>>>> SPECINT2000 is a bit worse (0.5%) mainly because of (> 3%) vortex
>>>>> degradation.  IMHO, vortex should be ignored unless profile-based
>>>>> optimizations are used because it contains hot loop containing only if
>>>>> with
>>>>> small then-part and big else-part and most time only the then-part is
>>>>> executed.
>>>>>
>>>>> SPECFP2000 is 0.6% better but taking off art volatility (I am having it
>>>>> on
>>>>> 10-15% on power6 and I don't know why), SPECFP2000 is about 2% better
>>>>> in
>>>>> overall with 9% improvement on mgrid, almost 3% on mesa, galgel and
>>>>> fma3d
>>>>> and 1.5% on equake and apsi.
>>>>>
>>>>> So I'd definitely remove the constraint for register-pressure sensitive
>>>>> insn
>>>>> scheduling using
>>>>>
>>>>> if (!reload_completed && !flag_sched_pressure)
>>>>>  return 1;
>>>>>
>>>>>
>>>>
>>>> The patch likelky caused PR41241, bootstrap comparison failures on hosts
>>>> that have address randomization enabled.  I can reproduce differences in
>>>> code-generation with re-building pieces of the 32bit multilib of libgcc
>>>> with the stage2 compiler.  -g doesn't seem to be necessary (not that
>>>> this
>>>> proves it isn't a VTA issue in the end).  One bootstrap with this patch
>>>> reverted has succeeded (but that's no prove either - only two out of
>>>> three
>>>> bootstraps fail for me).
>>>>
>>>>
>>>
>>> Richard, thanks for working on this.  To be honest, I have no idea what
>>> is
>>> wrong  with my patch because I have not enabled register pressure
>>> -sensitive
>>> insn scheduling yet by default.  In theory IRA and scheduler should work
>>> as
>>> before by default.  But may be I missed something (there are some very
>>> minor
>>> changes in non register pressure-sensitive scheduling part.  I should
>>> check
>>> them).
>>>
>>
>> Valgrind reports:
>>
>> ==7192== Invalid read of size 1
>> ==7192==    at 0x7291F8: reg_preferred_class (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6A95AC: rest_of_handle_ira (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF461: execute_one_pass (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF6B4: execute_pass_list (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF6C6: execute_pass_list (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x7F677B: tree_rest_of_compilation (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x963B5B: cgraph_expand_function (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x965AD4: cgraph_finalize_compilation_unit (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==  Address 0x5143a1f is 0 bytes after a block of size 495 alloc'd
>> ==7192==    at 0x4A1EEFC: calloc (in
>> /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
>> ==7192==    by 0xC38119: xcalloc (in /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x7276F4: reginfo_init (in /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF461: execute_one_pass (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF6B4: execute_pass_list (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x6FF6C6: execute_pass_list (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x7F677B: tree_rest_of_compilation (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x963B5B: cgraph_expand_function (in
>> /space/rguenther/obj/gcc/cc1)
>> ==7192==    by 0x965AD4: cgraph_finalize_compilation_unit (in
>> /space/rguenther/obj/gcc/cc1)
>>
>> maybe that's it.  Does it ring a bell?
>>
>>
>
> Sure, it is wrong.  It should be fixed.  Could you send the test case.

Attached to the PR.

Richard.