Re: [PATCH, SMS] Fix violation of memory dependence

[Ayal Zaks <ZAKS@il.ibm.com>]

Revital Eres <revital.eres@linaro.org> wrote on 13/06/2011 10:29:06 AM:

> From: Revital Eres <revital.eres@linaro.org>
> To: Ayal Zaks/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org, Patch Tracking <patches@linaro.org>
> Date: 13/06/2011 10:29 AM
> Subject: [PATCH, SMS] Fix violation of memory dependence
>
> Hello,
>
> The attached patch fixes violation of memory dependencies. The
> problematic scenario happens when -fmodulo-sched-allow-regmoves flag
> is set and certain anti-dep edges are not created.
>
> For example, consider the following three instructions and the edges
> between them.  When -fmodulo-sched-allow-regmoves is set the edge (63 -
> Anti, 0 -> 64) is not created. (probably due to transitivity)
>
> Insn 63)  r168 = MEM[176]
> Out edges: (63 - Anti, 0 -> 64)
> In edges: (64 - True, 1 -> 63), (68 - True, 1 -> 63)
>
> insn 64)  176 = 176 + 4
> Out edges: (64 - True, 1 -> 63), (64 - True, 0-> 68)
> In edges: (63 - Anti, 0 -> 64)
>
> insn 68)  MEM[176 – 4] =  193
> Out edges: (68 - True, 1 -> 63)
> In edges: (64 - True, 0-> 68)
>
> This anti-dep edge is on the path from one memory instruction to another
> --- from 63 to 68; such that removing the edge caused a violation of
> the memory dependencies as insn 63 was scheduled after insn 68.
>
> This patch adds intra edges between every two memory instructions in
> this case.  It fixes recent bootstrap failure on ARM. (with SMS flags)
>
> The patch was tested as follows:
> On ppc64-redhat-linux regtest as well as bootstrap with SMS flags
> enabling SMS also on loops with stage count 1.  Regtested on SPU.
> On arm-linux-gnueabi bootstrap c language with SMS
> flags enabling SMS also on loops with stage count 1
> and currently regression testing on c,c++.
>
> OK for mainline once regtest on arm-linux-gnueabi completes?
>

Yes, this is a straightforward fix to a wrong-code bug, as discussed
offline. Other alternatives that might introduce less edges:
o connect predecessors of u with v, and u with successors of v, when
removing edge (u,v). Maybe there are other cases which rely on transitivity
(?).
o have a version of sched_analyze that avoids creating register anti-deps
to begin with, and thus will create memory-deps in the absence of
transitivity.


>>         * ddg.c (add_intra_loop_mem_dep): New function.

You could check first thing if (from->cuid == to->cuid), for code clarity.

Nice catch,
Ayal.


> Thanks,
> Revital
>
> Changelog:
>
> gcc/
>         * ddg.c (add_intra_loop_mem_dep): New function.
>         (build_intra_loop_deps): Call it.
>
> testsuite/
>         * gcc.dg/sms-9.c: New file.
> [attachment "patch_fix_regmoves_12_6.txt" deleted by Ayal Zaks/Haifa/IBM]