| Message ID | 4C6B9A2F.4090805@ispras.ru |
|---|---|
| State | New |
| Headers | show |
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.
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?
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
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.
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
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
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"?
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
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