Revital Eres <revital.e...@linaro.org> wrote on 13/06/2011 10:29:06 AM:
> From: Revital Eres <revital.e...@linaro.org> > To: Ayal Zaks/Haifa/IBM@IBMIL > Cc: gcc-patches@gcc.gnu.org, Patch Tracking <patc...@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]
