* [PATCH] Fix PR 44691
@ 2010-08-18 8:53 Andrey Belevantsev
2010-08-18 16:34 ` Vladimir Makarov
2010-08-19 14:11 ` H.J. Lu
0 siblings, 2 replies; 9+ messages in thread
From: Andrey Belevantsev @ 2010-08-18 8:53 UTC (permalink / raw)
To: GCC Patches; +Cc: Vladimir N. Makarov
[-- Attachment #1: Type: text/plain, Size: 768 bytes --]
Hello,
As explained in the audit trail, the problem was that in the selective
scheduler I assumed that SUBREG_REG will always be a REG, which seems to be
not the case. This is not quite in line with what documentation says, if I
read it correctly, but it seems to be used in a number of backends, so the
below patch just gives up substitution also when SUBREG_REG is not a
register. Bootstrapped and tested on ia64, and verified that the test is
fixed on x86_64.
I think that this qualifies as obvious, so unless Vlad or other people have
any comments, I'll commit it tomorrow.
Yours, Andrey
2010-08-18 Andrey Belevantsev <abel@ispras.ru>
PR rtl-optimization/44691
* sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
is not a register.
[-- Attachment #2: pr44691.diff --]
[-- Type: text/x-patch, Size: 858 bytes --]
Index: gcc/sel-sched.c
===================================================================
*** gcc/sel-sched.c (revision 163192)
--- gcc/sel-sched.c (working copy)
*************** count_occurrences_1 (rtx *cur_rtx, void
*** 835,841 ****
if (GET_CODE (*cur_rtx) == SUBREG
&& REG_P (p->x)
! && REGNO (SUBREG_REG (*cur_rtx)) == REGNO (p->x))
{
/* ??? Do not support substituting regs inside subregs. In that case,
simplify_subreg will be called by validate_replace_rtx, and
--- 835,842 ----
if (GET_CODE (*cur_rtx) == SUBREG
&& REG_P (p->x)
! && (!REG_P (SUBREG_REG (*cur_rtx))
! || REGNO (SUBREG_REG (*cur_rtx)) == REGNO (p->x)))
{
/* ??? Do not support substituting regs inside subregs. In that case,
simplify_subreg will be called by validate_replace_rtx, and
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-18 8:53 [PATCH] Fix PR 44691 Andrey Belevantsev
@ 2010-08-18 16:34 ` Vladimir Makarov
2010-08-19 14:11 ` H.J. Lu
1 sibling, 0 replies; 9+ messages in thread
From: Vladimir Makarov @ 2010-08-18 16:34 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches
On 08/18/2010 04:30 AM, Andrey Belevantsev wrote:
> Hello,
>
> As explained in the audit trail, the problem was that in the selective
> scheduler I assumed that SUBREG_REG will always be a REG, which seems
> to be not the case. This is not quite in line with what documentation
> says, if I read it correctly, but it seems to be used in a number of
> backends, so the below patch just gives up substitution also when
> SUBREG_REG is not a register. Bootstrapped and tested on ia64, and
> verified that the test is fixed on x86_64.
>
> I think that this qualifies as obvious, so unless Vlad or other people
> have any comments, I'll commit it tomorrow.
>
Yes, it is obvious.
> Yours, Andrey
>
> 2010-08-18 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/44691
>
> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
> is not a register.
>
Ok to commit. Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-18 8:53 [PATCH] Fix PR 44691 Andrey Belevantsev
2010-08-18 16:34 ` Vladimir Makarov
@ 2010-08-19 14:11 ` H.J. Lu
2010-08-19 14:14 ` Andrey Belevantsev
1 sibling, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2010-08-19 14:11 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches, Vladimir N. Makarov
2010/8/18 Andrey Belevantsev <abel@ispras.ru>:
> Hello,
>
> As explained in the audit trail, the problem was that in the selective
> scheduler I assumed that SUBREG_REG will always be a REG, which seems to be
> not the case. This is not quite in line with what documentation says, if I
> read it correctly, but it seems to be used in a number of backends, so the
> below patch just gives up substitution also when SUBREG_REG is not a
> register. Bootstrapped and tested on ia64, and verified that the test is
> fixed on x86_64.
>
> I think that this qualifies as obvious, so unless Vlad or other people have
> any comments, I'll commit it tomorrow.
>
> Yours, Andrey
>
> 2010-08-18 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/44691
>
> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
> is not a register.
>
Shouldn't we add the testcase?
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 14:11 ` H.J. Lu
@ 2010-08-19 14:14 ` Andrey Belevantsev
2010-08-19 14:24 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Belevantsev @ 2010-08-19 14:14 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Vladimir N. Makarov
On 19.08.2010 17:28, H.J. Lu wrote:
> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>> Hello,
>>
>> As explained in the audit trail, the problem was that in the selective
>> scheduler I assumed that SUBREG_REG will always be a REG, which seems to be
>> not the case. This is not quite in line with what documentation says, if I
>> read it correctly, but it seems to be used in a number of backends, so the
>> below patch just gives up substitution also when SUBREG_REG is not a
>> register. Bootstrapped and tested on ia64, and verified that the test is
>> fixed on x86_64.
>>
>> I think that this qualifies as obvious, so unless Vlad or other people have
>> any comments, I'll commit it tomorrow.
>>
>> Yours, Andrey
>>
>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>
>> PR rtl-optimization/44691
>>
>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>> is not a register.
>>
>
> Shouldn't we add the testcase?
The test is fortran.dg/pr42294.f which is actually mentioned in the bug
report. Sorry for not saying this explicitly in the mail.
Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 14:14 ` Andrey Belevantsev
@ 2010-08-19 14:24 ` H.J. Lu
2010-08-19 14:37 ` Andrey Belevantsev
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2010-08-19 14:24 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches, Vladimir N. Makarov
2010/8/19 Andrey Belevantsev <abel@ispras.ru>:
> On 19.08.2010 17:28, H.J. Lu wrote:
>>
>> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>>>
>>> Hello,
>>>
>>> As explained in the audit trail, the problem was that in the selective
>>> scheduler I assumed that SUBREG_REG will always be a REG, which seems to
>>> be
>>> not the case. This is not quite in line with what documentation says, if
>>> I
>>> read it correctly, but it seems to be used in a number of backends, so
>>> the
>>> below patch just gives up substitution also when SUBREG_REG is not a
>>> register. Bootstrapped and tested on ia64, and verified that the test is
>>> fixed on x86_64.
>>>
>>> I think that this qualifies as obvious, so unless Vlad or other people
>>> have
>>> any comments, I'll commit it tomorrow.
>>>
>>> Yours, Andrey
>>>
>>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>>
>>> PR rtl-optimization/44691
>>>
>>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>>> is not a register.
>>>
>>
>> Shouldn't we add the testcase?
>
> The test is fortran.dg/pr42294.f which is actually mentioned in the bug
> report. Sorry for not saying this explicitly in the mail.
>
Normally this bug isn't trigged. You need to pass -O2 -fselective-scheduling2
to see it. You should copy gfortran.dg/pr42294.f and add -O2
-fselective-scheduling2.
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 14:24 ` H.J. Lu
@ 2010-08-19 14:37 ` Andrey Belevantsev
2010-08-19 15:53 ` Andrey Belevantsev
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Belevantsev @ 2010-08-19 14:37 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Vladimir N. Makarov
On 19.08.2010 18:12, H.J. Lu wrote:
> 2010/8/19 Andrey Belevantsev<abel@ispras.ru>:
>> On 19.08.2010 17:28, H.J. Lu wrote:
>>>
>>> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>>>>
>>>> Hello,
>>>>
>>>> As explained in the audit trail, the problem was that in the selective
>>>> scheduler I assumed that SUBREG_REG will always be a REG, which seems to
>>>> be
>>>> not the case. This is not quite in line with what documentation says, if
>>>> I
>>>> read it correctly, but it seems to be used in a number of backends, so
>>>> the
>>>> below patch just gives up substitution also when SUBREG_REG is not a
>>>> register. Bootstrapped and tested on ia64, and verified that the test is
>>>> fixed on x86_64.
>>>>
>>>> I think that this qualifies as obvious, so unless Vlad or other people
>>>> have
>>>> any comments, I'll commit it tomorrow.
>>>>
>>>> Yours, Andrey
>>>>
>>>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>>>
>>>> PR rtl-optimization/44691
>>>>
>>>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>>>> is not a register.
>>>>
>>>
>>> Shouldn't we add the testcase?
>>
>> The test is fortran.dg/pr42294.f which is actually mentioned in the bug
>> report. Sorry for not saying this explicitly in the mail.
>>
>
> Normally this bug isn't trigged. You need to pass -O2 -fselective-scheduling2
> to see it. You should copy gfortran.dg/pr42294.f and add -O2
> -fselective-scheduling2.
Ah, ok, I forgot about the explicit options. I will do that.
Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 14:37 ` Andrey Belevantsev
@ 2010-08-19 15:53 ` Andrey Belevantsev
2010-08-19 16:03 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Belevantsev @ 2010-08-19 15:53 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Vladimir N. Makarov
On 19.08.2010 18:14, Andrey Belevantsev wrote:
> On 19.08.2010 18:12, H.J. Lu wrote:
>> 2010/8/19 Andrey Belevantsev<abel@ispras.ru>:
>>> On 19.08.2010 17:28, H.J. Lu wrote:
>>>>
>>>> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>>>>>
>>>>> Hello,
>>>>>
>>>>> As explained in the audit trail, the problem was that in the selective
>>>>> scheduler I assumed that SUBREG_REG will always be a REG, which seems to
>>>>> be
>>>>> not the case. This is not quite in line with what documentation says, if
>>>>> I
>>>>> read it correctly, but it seems to be used in a number of backends, so
>>>>> the
>>>>> below patch just gives up substitution also when SUBREG_REG is not a
>>>>> register. Bootstrapped and tested on ia64, and verified that the test is
>>>>> fixed on x86_64.
>>>>>
>>>>> I think that this qualifies as obvious, so unless Vlad or other people
>>>>> have
>>>>> any comments, I'll commit it tomorrow.
>>>>>
>>>>> Yours, Andrey
>>>>>
>>>>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>>>>
>>>>> PR rtl-optimization/44691
>>>>>
>>>>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>>>>> is not a register.
>>>>>
>>>>
>>>> Shouldn't we add the testcase?
>>>
>>> The test is fortran.dg/pr42294.f which is actually mentioned in the bug
>>> report. Sorry for not saying this explicitly in the mail.
>>>
>>
>> Normally this bug isn't trigged. You need to pass -O2
>> -fselective-scheduling2
>> to see it. You should copy gfortran.dg/pr42294.f and add -O2
>> -fselective-scheduling2.
> Ah, ok, I forgot about the explicit options. I will do that.
Looking closely, pr42294.f happens to be another sel-sched bug, so it
already has "-O2 -fselective-scheduling2 -fsel-sched-pipelining
-funroll-all-loops" as dg-options. So I guess this test should be enough,
what do you think?
Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 15:53 ` Andrey Belevantsev
@ 2010-08-19 16:03 ` H.J. Lu
2010-08-20 8:42 ` Andrey Belevantsev
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2010-08-19 16:03 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches, Vladimir N. Makarov
2010/8/19 Andrey Belevantsev <abel@ispras.ru>:
> On 19.08.2010 18:14, Andrey Belevantsev wrote:
>>
>> On 19.08.2010 18:12, H.J. Lu wrote:
>>>
>>> 2010/8/19 Andrey Belevantsev<abel@ispras.ru>:
>>>>
>>>> On 19.08.2010 17:28, H.J. Lu wrote:
>>>>>
>>>>> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> As explained in the audit trail, the problem was that in the selective
>>>>>> scheduler I assumed that SUBREG_REG will always be a REG, which seems
>>>>>> to
>>>>>> be
>>>>>> not the case. This is not quite in line with what documentation says,
>>>>>> if
>>>>>> I
>>>>>> read it correctly, but it seems to be used in a number of backends, so
>>>>>> the
>>>>>> below patch just gives up substitution also when SUBREG_REG is not a
>>>>>> register. Bootstrapped and tested on ia64, and verified that the test
>>>>>> is
>>>>>> fixed on x86_64.
>>>>>>
>>>>>> I think that this qualifies as obvious, so unless Vlad or other people
>>>>>> have
>>>>>> any comments, I'll commit it tomorrow.
>>>>>>
>>>>>> Yours, Andrey
>>>>>>
>>>>>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>>>>>
>>>>>> PR rtl-optimization/44691
>>>>>>
>>>>>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>>>>>> is not a register.
>>>>>>
>>>>>
>>>>> Shouldn't we add the testcase?
>>>>
>>>> The test is fortran.dg/pr42294.f which is actually mentioned in the bug
>>>> report. Sorry for not saying this explicitly in the mail.
>>>>
>>>
>>> Normally this bug isn't trigged. You need to pass -O2
>>> -fselective-scheduling2
>>> to see it. You should copy gfortran.dg/pr42294.f and add -O2
>>> -fselective-scheduling2.
>>
>> Ah, ok, I forgot about the explicit options. I will do that.
>
> Looking closely, pr42294.f happens to be another sel-sched bug, so it
> already has "-O2 -fselective-scheduling2 -fsel-sched-pipelining
> -funroll-all-loops" as dg-options. So I guess this test should be enough,
> what do you think?
Why didn't it fail before? Does this bug fail only with "-O2
-fselective-scheduling2"?
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix PR 44691
2010-08-19 16:03 ` H.J. Lu
@ 2010-08-20 8:42 ` Andrey Belevantsev
0 siblings, 0 replies; 9+ messages in thread
From: Andrey Belevantsev @ 2010-08-20 8:42 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Vladimir N. Makarov
On 19.08.2010 19:53, H.J. Lu wrote:
> 2010/8/19 Andrey Belevantsev<abel@ispras.ru>:
>> On 19.08.2010 18:14, Andrey Belevantsev wrote:
>>>
>>> On 19.08.2010 18:12, H.J. Lu wrote:
>>>>
>>>> 2010/8/19 Andrey Belevantsev<abel@ispras.ru>:
>>>>>
>>>>> On 19.08.2010 17:28, H.J. Lu wrote:
>>>>>>
>>>>>> 2010/8/18 Andrey Belevantsev<abel@ispras.ru>:
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> As explained in the audit trail, the problem was that in the selective
>>>>>>> scheduler I assumed that SUBREG_REG will always be a REG, which seems
>>>>>>> to
>>>>>>> be
>>>>>>> not the case. This is not quite in line with what documentation says,
>>>>>>> if
>>>>>>> I
>>>>>>> read it correctly, but it seems to be used in a number of backends, so
>>>>>>> the
>>>>>>> below patch just gives up substitution also when SUBREG_REG is not a
>>>>>>> register. Bootstrapped and tested on ia64, and verified that the test
>>>>>>> is
>>>>>>> fixed on x86_64.
>>>>>>>
>>>>>>> I think that this qualifies as obvious, so unless Vlad or other people
>>>>>>> have
>>>>>>> any comments, I'll commit it tomorrow.
>>>>>>>
>>>>>>> Yours, Andrey
>>>>>>>
>>>>>>> 2010-08-18 Andrey Belevantsev<abel@ispras.ru>
>>>>>>>
>>>>>>> PR rtl-optimization/44691
>>>>>>>
>>>>>>> * sel-sched.c (count_occurrences_1): Also punt when SUBREG_REG
>>>>>>> is not a register.
>>>>>>>
>>>>>>
>>>>>> Shouldn't we add the testcase?
>>>>>
>>>>> The test is fortran.dg/pr42294.f which is actually mentioned in the bug
>>>>> report. Sorry for not saying this explicitly in the mail.
>>>>>
>>>>
>>>> Normally this bug isn't trigged. You need to pass -O2
>>>> -fselective-scheduling2
>>>> to see it. You should copy gfortran.dg/pr42294.f and add -O2
>>>> -fselective-scheduling2.
>>>
>>> Ah, ok, I forgot about the explicit options. I will do that.
>>
>> Looking closely, pr42294.f happens to be another sel-sched bug, so it
>> already has "-O2 -fselective-scheduling2 -fsel-sched-pipelining
>> -funroll-all-loops" as dg-options. So I guess this test should be enough,
>> what do you think?
>
> Why didn't it fail before? Does this bug fail only with "-O2
> -fselective-scheduling2"?
As mentioned in the PR, it started to fail with addition of the lea splits.
But you are right, you need to remove -funroll-loops from the compile
options for the test to fail, so I have added a copy with just -O2
-fselective-scheduling2 and committed as 163396.
Thanks for noticing,
Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-08-20 8:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-18 8:53 [PATCH] Fix PR 44691 Andrey Belevantsev
2010-08-18 16:34 ` Vladimir Makarov
2010-08-19 14:11 ` H.J. Lu
2010-08-19 14:14 ` Andrey Belevantsev
2010-08-19 14:24 ` H.J. Lu
2010-08-19 14:37 ` Andrey Belevantsev
2010-08-19 15:53 ` Andrey Belevantsev
2010-08-19 16:03 ` H.J. Lu
2010-08-20 8:42 ` Andrey Belevantsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).