[SMS] Schedule normalization after scheduling branch

Wed Jan 20 21:30:00 GMT 2016

On 01/18/2016 02:38 PM, Roman Zhuykov wrote:
> Hello,
>
> 4 years ago when I create some SMS patches nobody cares about SMS
> failures on ia64.  Now ia64 is even more dead but at least one of bugs
> appears on powerpc - PR69252.
> Proposed patch is here:
> https://gcc.gnu.org/ml/gcc-patches/2011-12/txt00266.txt and it even
> suits current trunk without modification.
>
> Powerpc PR69252 situation seems to be the same as I described earlier
> on ia64, I can discuss it once again.
>
> There were the following doloop:
>
>    set_before reg
> Loop:
>    use1 reg
>    set reg
>    use2 reg
>    insn
>    cloop
>
> After SMS it looked like this (I add scheduling stage and cycle before
> each loop instruction):
>
>    set_before reg
> SMS-prologue:
>    set reg
>    use1 reg_copy #uninitialized
>    use2 reg
>    reg_copy <- reg
> Loop:
> 0  0 set reg
> 0  0 use1 reg_copy
> 0  4 use2 reg
> 0 -4 reg_copy <- reg
> 0  8 insn
> 0 -1 cloop
>
> All instructions were wrongly classified to stage zero.  While copying
> them to prologue the regmove remains to be placed after use1, and as a
> result, the register reg_copy is used uninititalized in prologue.
> This leads to miscompilation.
>
> I have found that the issue can be fixed by additional schedule
> normalizarion after scheduling branch instruction in optimize_sc
> function.  The situation here is the same as in patch by Richard
> Sandiford
> http://gcc.gnu.org/ml/gcc-patches/2011-10/msg00748.html which enables
> scheduling regmoves.  "Moves that handle incoming values might have
> been added to a new first stage.  Bump the stage count if so."  The
> same bumping should be done after scheduling branch, and my patch
> simply implements this.
>
> As Martin Sebor have already done bootstrap and regtest in PR69252, I
> want to ask him to attach PR69252 example as a new regression test to
> this patch.  I don't know if it should be execution test or maybe
> there are some special scan-assembler techniques.
>
> Ok for trunk with regtest?

A patch with the regression test is attached.  I verified that
the test fails without your patch and passes with it applied.

Martin

>
> --
> Roman
>
> Here is the whole old letter about the patch:
> 2011-12-29 17:04 GMT+03:00 Roman Zhuykov <zhroma@ispras.ru>:
>> This week I investigated modulo scheduler on IA64.  Enabling SMS by default
>> (-fmodulo-sched -fmodulo-sched-allow-regmoves) leads to bootstrap failure
>> on IA64: gcc/build/genautomata.o differs while comparing stages 2 and 3.
>>
>> I haven't studied this issue in detail, because the combination of these
>> my patches fixes this problem:
>>
>> [Additional edges to instructions with clobber]
>> http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00505.html
>> [Correctly delete anti-dep edges for renaming]
>> http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00506.html
>>
>> Then I have regtested two compilers - first is clean trunk, and the second
>> is trunk with SMS enabled by default and two patches mentioned above.
>> Comparing the results shows several new failures.
>>
>> FAIL: gcc.dg/pr45259.c (internal compiler error)
>> FAIL: gcc.dg/pr45259.c (test for excess errors)
>> FAIL: gcc.dg/pr47881.c (test for excess errors)
>> FAIL: tr1/5_numerical_facilities/special_functions/08_cyl_bessel_i/check_value.cc
>> execution test
>> FAIL: tr1/5_numerical_facilities/special_functions/09_cyl_bessel_j/check_value.cc
>> execution test
>> FAIL: tr1/5_numerical_facilities/special_functions/11_cyl_neumann/check_value.cc
>> execution test
>> FAIL: tr1/5_numerical_facilities/special_functions/21_sph_bessel/check_value.cc
>> execution test
>> FAIL: tr1/5_numerical_facilities/special_functions/23_sph_neumann/check_value.cc
>> execution test
>>
>> Problem with gcc.dg/pr45259.c is an ICE, which I earlier fixed by this patch:
>> [Correct extracting loop exit condition]
>> http://gcc.gnu.org/ml/gcc-patches/2011-09/msg02049.html
>>
>> In gcc.dg/pr47881.c -fcompare-debug failure happens. The difference between
>> -fcompare-debug dumps is only some NOTE_INSN_DELETED entries are placed
>> differently.  I haven't studied this problem.
>>
>> And the last 5 new failures have dissappered after fixing the following
>> described issue.
>>
>> Imagine the following doloop (each use and set is a fmad in real example):
>>
>> use1 reg
>> set reg
>> use2 reg
>> insn
>> cloop
>>
>> After SMS it looks like this, I write a scheduling stage and cycle before each
>> instruction.
>>
>> 0  0 set reg
>> 0  0 use1 reg_copy
>> 0  4 use2 regR
>> 0 -4 reg_copy = reg
>> 0  8 insn
>> 0 -1 cloop
>>
>> So all instructions were wrongly classified to stage zero.  While copying them
>> to prologue the regmove remains to be placed after use1, and as a
>> result, the register
>> reg_copy is used uninititalized in prologue.  This leads to miscompilation.
>>
>> I have found that the issue can be fixed by additional schedule normalizarion
>> after scheduling branch instruction in optimize_sc function.  The situation
>> here is the same as in patch by Richard Sandiford
>> http://gcc.gnu.org/ml/gcc-patches/2011-10/msg00748.html
>> which enables scheduling regmoves.
>> "Moves that handle incoming values might have been added
>> to a new first stage.  Bump the stage count if so."
>> The same bumping should be done after scheduling branch.
>>
>> In my model example branch is scheduled on cycle -1 and remains in zero stage.
>> When regmove is later scheduled on cycle -4 the Richard's check doesn't cause
>> normalization, because new PS_MIN_CYCLE is -4, but min_cycle is -12:
>>
>>    min_cycle = PS_MIN_CYCLE (ps) - SMODULO (PS_MIN_CYCLE (ps), ps->ii);
>>    ...
>>    call schedule_reg_moves (ps)
>>    ...
>>    if (PS_MIN_CYCLE (ps) < min_cycle)
>>      {
>>        reset_sched_times (ps, 0);
>>        stage_count++;
>>      }
>>
>> I attach patch which adds the same check into optimize_sc function.
>> With -fmodulo-sched -fmodulo-sched-allow-regmoves enabled it passes
>> bootstrap and regtest on IA64.  It also passes bootstrap and regtest
>> on x86_64 with SMS patched to schedule non-doloop loops.
>> [Support new loop pattern]
>> http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00495.html
>>
>> OK for trunk or maybe 4.8?
>>
>> Happy holidays!
>>
>> --
>> Roman Zhuykov

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-69252-test.patch
Type: text/x-patch
Size: 935 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160120/83d62b67/attachment.bin>