On Wed, May 29, 2013 at 9:36 AM, Dinar Temirbulatov
<di...@kugelworks.com> wrote:
> Here is the corrected version of change. Also, I think, I need
> write-after-approval access to commit the change.
> thanks, Dinar,
>
> PR rtl-optimization/57268
> * sched-deps.c (sched_analyze_2): Flush dependence lists if
> the sum of the read and write lists exceeds MAX_PENDING_LIST_LENGTH.
>
> On Wed, May 29, 2013 at 6:49 PM, Jeff Law <l...@redhat.com> wrote:
>> On 05/29/2013 06:52 AM, Steven Bosscher wrote:
>>>
>>> Hello,
>>>
>>> On Wed, May 29, 2013 at 2:01 PM, Dinar Temirbulatov wrote:
>>>>
>>>> Hi,
>>>> I noticed that the scheduler created long dependence list about ~9000
>>>> elements long and slowed compilation time for about an hour. Attached
>>>> patch flushes the dependence list is case of it is longer than
>>>> MAX_PENDING_LIST_LENGTH. Tested with gcc testsite on x86_64-linux-gnu
>>>> with c and c++ enabled. Ok for trunk?
>>>> thanks, Dinar.
>>>>
>>>> 2013-05-28 Dinar Temirbulatov <dinar at kugelworks dot com>
>>>>
>>>> PR rtl-optimization/57268
>>>> * sched-deps.c (sched_analyze_2): Flush dependence list
>>>> then it is longer than MAX_PENDING_LIST_LENGTH.
>>>
>>>
>>>
>>> * sched-deps.c (sched_analyze_2): Flush dependence lists if
>>> the sum of the read and write lists exceeds
>>> MAX_PENDING_LIST_LENGTH.
>>>
>>>
>>>
>>>> if (!deps->readonly)
>>>> - add_insn_mem_dependence (deps, true, insn, x);
>>>> + {
>>>> + if ((deps->pending_read_list_length +
>>>> deps->pending_write_list_length)
>>>> + > MAX_PENDING_LIST_LENGTH)
>>>> + flush_pending_lists (deps, insn, true, true);
>>>> + add_insn_mem_dependence (deps, true, insn, x);
>>>> + }
>>>
>>>
>>> The "flush_pending_lists", "add_insn_mem_dependence" and "}" lines are
>>> not indented correctly. The if (...+...) line is too long (max. 80
>>> characters per line). The GCC style would be
>>>
>>> if (!deps->readonly)
>>> {
>>> if ((deps->pending_read_list_length
>>> + deps->pending_write_list_length)
>>> > MAX_PENDING_LIST_LENGTH)
>>> flush_pending_lists (deps, insn, true, true);
>>> add_insn_mem_dependence (deps, true, insn, x);
>>> }
>>>
>>> (The aesthetics of GCC code style is a matter for debate, but not here
>>> and now ;-)
>>
>> And just to be clear, with Steven's suggested changes, this patch is OK.
>>
>> jeff
This caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57494
--
H.J.
