| Message ID | 1477582641.17912.136.camel@brimstone.rchland.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 27, 2016 at 5:37 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > Hi, > > Per PR72747, A statement such as "v = vec_splats (1);" correctly > initializes a vector. However, a statement such as "v[1] = v[0] = > vec_splats (1);" initializes both v[1] and v[0] to random garbage. > > It has been determined that this is occurring because we did not emit > the actual initialization statement before our final exit from > gimplify_init_constructor, at which time we lose the expression when we > assign *expr_p to either NULL or object. This problem affected both constant > and non-constant initializers. Corrected this by moving the logic to > emit the statement up earlier within the if/else logic. > > Bootstrapped and make check ran without regressions on > powerpc64le-unknown-linux-gnu. > > OK for trunk? Ok. RIchard. > Thanks, > -Will > > gcc: > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > PR middle-end/72747 > * gimplify.c (gimplify_init_constructor): Move emit of constructor > assignment to earlier in the if/else logic. > > testsuite: > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > PR middle-end/72747 > * c-c++-common/pr72747-1.c: New test. > * c-c++-common/pr72747-2.c: Likewise. > > Index: gcc/gimplify.c > =================================================================== > --- gcc/gimplify.c (revision 241600) > +++ gcc/gimplify.c (working copy) > @@ -4730,24 +4730,23 @@ > > if (ret == GS_ERROR) > return GS_ERROR; > - else if (want_value) > + /* If we have gimplified both sides of the initializer but have > + not emitted an assignment, do so now. */ > + if (*expr_p) > { > + tree lhs = TREE_OPERAND (*expr_p, 0); > + tree rhs = TREE_OPERAND (*expr_p, 1); > + gassign *init = gimple_build_assign (lhs, rhs); > + gimplify_seq_add_stmt (pre_p, init); > + } > + if (want_value) > + { > *expr_p = object; > return GS_OK; > } > else > { > - /* If we have gimplified both sides of the initializer but have > - not emitted an assignment, do so now. */ > - if (*expr_p) > - { > - tree lhs = TREE_OPERAND (*expr_p, 0); > - tree rhs = TREE_OPERAND (*expr_p, 1); > - gassign *init = gimple_build_assign (lhs, rhs); > - gimplify_seq_add_stmt (pre_p, init); > - *expr_p = NULL; > - } > - > + *expr_p = NULL; > return GS_ALL_DONE; > } > } > Index: gcc/testsuite/c-c++-common/pr72747-1.c > =================================================================== > --- gcc/testsuite/c-c++-common/pr72747-1.c (revision 0) > +++ gcc/testsuite/c-c++-common/pr72747-1.c (working copy) > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target powerpc_altivec_ok } */ > +/* { dg-options "-maltivec -fdump-tree-gimple" } */ > + > +/* PR 72747: Test that cascaded definition is happening for constant vectors. */ > + > +#include <altivec.h> > + > +int main (int argc, char *argv[]) > +{ > + __vector int v1,v2; > + v1 = v2 = vec_splats ((int) 42); > + return 0; > +} > +/* { dg-final { scan-tree-dump-times " v2 = { 42, 42, 42, 42 }" 1 "gimple" } } */ > + > Index: gcc/testsuite/c-c++-common/pr72747-2.c > =================================================================== > --- gcc/testsuite/c-c++-common/pr72747-2.c (revision 0) > +++ gcc/testsuite/c-c++-common/pr72747-2.c (working copy) > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target powerpc_altivec_ok } */ > +/* { dg-options "-c -maltivec -fdump-tree-gimple" } */ > + > +/* PR 72747: test that cascaded definition is happening for non constants. */ > + > +void foo () > +{ > + extern int i; > + __vector int v,w; > + v = w = (vector int) { i }; > +} > + > +int main (int argc, char *argv[]) > +{ > + return 0; > +} > +/* { dg-final { scan-tree-dump-times " w = {i.0_1}" 1 "gimple" } } */ > >
On Fri, 2016-10-28 at 10:38 +0200, Richard Biener wrote: > On Thu, Oct 27, 2016 at 5:37 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > > Hi, > > > > Per PR72747, A statement such as "v = vec_splats (1);" correctly > > initializes a vector. However, a statement such as "v[1] = v[0] = > > vec_splats (1);" initializes both v[1] and v[0] to random garbage. > > > > It has been determined that this is occurring because we did not emit > > the actual initialization statement before our final exit from > > gimplify_init_constructor, at which time we lose the expression when we > > assign *expr_p to either NULL or object. This problem affected both constant > > and non-constant initializers. Corrected this by moving the logic to > > emit the statement up earlier within the if/else logic. > > > > Bootstrapped and make check ran without regressions on > > powerpc64le-unknown-linux-gnu. > > > > OK for trunk? > > Ok. > > RIchard. Committed revision 241647. Thanks. > > > Thanks, > > -Will > > > > gcc: > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > PR middle-end/72747 > > * gimplify.c (gimplify_init_constructor): Move emit of constructor > > assignment to earlier in the if/else logic. > > > > testsuite: > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > PR middle-end/72747 > > * c-c++-common/pr72747-1.c: New test. > > * c-c++-common/pr72747-2.c: Likewise. > > > > Index: gcc/gimplify.c > > =================================================================== > > --- gcc/gimplify.c (revision 241600) > > +++ gcc/gimplify.c (working copy) > > @@ -4730,24 +4730,23 @@ > > > > if (ret == GS_ERROR) > > return GS_ERROR; > > - else if (want_value) > > + /* If we have gimplified both sides of the initializer but have > > + not emitted an assignment, do so now. */ > > + if (*expr_p) > > { > > + tree lhs = TREE_OPERAND (*expr_p, 0); > > + tree rhs = TREE_OPERAND (*expr_p, 1); > > + gassign *init = gimple_build_assign (lhs, rhs); > > + gimplify_seq_add_stmt (pre_p, init); > > + } > > + if (want_value) > > + { > > *expr_p = object; > > return GS_OK; > > } > > else > > { > > - /* If we have gimplified both sides of the initializer but have > > - not emitted an assignment, do so now. */ > > - if (*expr_p) > > - { > > - tree lhs = TREE_OPERAND (*expr_p, 0); > > - tree rhs = TREE_OPERAND (*expr_p, 1); > > - gassign *init = gimple_build_assign (lhs, rhs); > > - gimplify_seq_add_stmt (pre_p, init); > > - *expr_p = NULL; > > - } > > - > > + *expr_p = NULL; > > return GS_ALL_DONE; > > } > > } > > Index: gcc/testsuite/c-c++-common/pr72747-1.c > > =================================================================== > > --- gcc/testsuite/c-c++-common/pr72747-1.c (revision 0) > > +++ gcc/testsuite/c-c++-common/pr72747-1.c (working copy) > > @@ -0,0 +1,16 @@ > > +/* { dg-do compile } */ > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > +/* { dg-options "-maltivec -fdump-tree-gimple" } */ > > + > > +/* PR 72747: Test that cascaded definition is happening for constant vectors. */ > > + > > +#include <altivec.h> > > + > > +int main (int argc, char *argv[]) > > +{ > > + __vector int v1,v2; > > + v1 = v2 = vec_splats ((int) 42); > > + return 0; > > +} > > +/* { dg-final { scan-tree-dump-times " v2 = { 42, 42, 42, 42 }" 1 "gimple" } } */ > > + > > Index: gcc/testsuite/c-c++-common/pr72747-2.c > > =================================================================== > > --- gcc/testsuite/c-c++-common/pr72747-2.c (revision 0) > > +++ gcc/testsuite/c-c++-common/pr72747-2.c (working copy) > > @@ -0,0 +1,18 @@ > > +/* { dg-do compile } */ > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > +/* { dg-options "-c -maltivec -fdump-tree-gimple" } */ > > + > > +/* PR 72747: test that cascaded definition is happening for non constants. */ > > + > > +void foo () > > +{ > > + extern int i; > > + __vector int v,w; > > + v = w = (vector int) { i }; > > +} > > + > > +int main (int argc, char *argv[]) > > +{ > > + return 0; > > +} > > +/* { dg-final { scan-tree-dump-times " w = {i.0_1}" 1 "gimple" } } */ > > > > >
On Fri, 2016-10-28 at 08:31 -0500, Will Schmidt wrote: > On Fri, 2016-10-28 at 10:38 +0200, Richard Biener wrote: > > On Thu, Oct 27, 2016 at 5:37 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > > > Hi, > > > > > > Per PR72747, A statement such as "v = vec_splats (1);" correctly > > > initializes a vector. However, a statement such as "v[1] = v[0] = > > > vec_splats (1);" initializes both v[1] and v[0] to random garbage. > > > > > > It has been determined that this is occurring because we did not emit > > > the actual initialization statement before our final exit from > > > gimplify_init_constructor, at which time we lose the expression when we > > > assign *expr_p to either NULL or object. This problem affected both constant > > > and non-constant initializers. Corrected this by moving the logic to > > > emit the statement up earlier within the if/else logic. > > > > > > Bootstrapped and make check ran without regressions on > > > powerpc64le-unknown-linux-gnu. > > > > > > OK for trunk? > > > > Ok. > > > > RIchard. > > Committed revision 241647. > Thanks. Oops, forgot to ask. After some burn-in time, is this OK to backport to the 5 and 6 branches? The bug was first noticed in GCC 5. Thanks, -Will > > > > > > > Thanks, > > > -Will > > > > > > gcc: > > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > > > PR middle-end/72747 > > > * gimplify.c (gimplify_init_constructor): Move emit of constructor > > > assignment to earlier in the if/else logic. > > > > > > testsuite: > > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > > > PR middle-end/72747 > > > * c-c++-common/pr72747-1.c: New test. > > > * c-c++-common/pr72747-2.c: Likewise. > > > > > > Index: gcc/gimplify.c > > > =================================================================== > > > --- gcc/gimplify.c (revision 241600) > > > +++ gcc/gimplify.c (working copy) > > > @@ -4730,24 +4730,23 @@ > > > > > > if (ret == GS_ERROR) > > > return GS_ERROR; > > > - else if (want_value) > > > + /* If we have gimplified both sides of the initializer but have > > > + not emitted an assignment, do so now. */ > > > + if (*expr_p) > > > { > > > + tree lhs = TREE_OPERAND (*expr_p, 0); > > > + tree rhs = TREE_OPERAND (*expr_p, 1); > > > + gassign *init = gimple_build_assign (lhs, rhs); > > > + gimplify_seq_add_stmt (pre_p, init); > > > + } > > > + if (want_value) > > > + { > > > *expr_p = object; > > > return GS_OK; > > > } > > > else > > > { > > > - /* If we have gimplified both sides of the initializer but have > > > - not emitted an assignment, do so now. */ > > > - if (*expr_p) > > > - { > > > - tree lhs = TREE_OPERAND (*expr_p, 0); > > > - tree rhs = TREE_OPERAND (*expr_p, 1); > > > - gassign *init = gimple_build_assign (lhs, rhs); > > > - gimplify_seq_add_stmt (pre_p, init); > > > - *expr_p = NULL; > > > - } > > > - > > > + *expr_p = NULL; > > > return GS_ALL_DONE; > > > } > > > } > > > Index: gcc/testsuite/c-c++-common/pr72747-1.c > > > =================================================================== > > > --- gcc/testsuite/c-c++-common/pr72747-1.c (revision 0) > > > +++ gcc/testsuite/c-c++-common/pr72747-1.c (working copy) > > > @@ -0,0 +1,16 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > > +/* { dg-options "-maltivec -fdump-tree-gimple" } */ > > > + > > > +/* PR 72747: Test that cascaded definition is happening for constant vectors. */ > > > + > > > +#include <altivec.h> > > > + > > > +int main (int argc, char *argv[]) > > > +{ > > > + __vector int v1,v2; > > > + v1 = v2 = vec_splats ((int) 42); > > > + return 0; > > > +} > > > +/* { dg-final { scan-tree-dump-times " v2 = { 42, 42, 42, 42 }" 1 "gimple" } } */ > > > + > > > Index: gcc/testsuite/c-c++-common/pr72747-2.c > > > =================================================================== > > > --- gcc/testsuite/c-c++-common/pr72747-2.c (revision 0) > > > +++ gcc/testsuite/c-c++-common/pr72747-2.c (working copy) > > > @@ -0,0 +1,18 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > > +/* { dg-options "-c -maltivec -fdump-tree-gimple" } */ > > > + > > > +/* PR 72747: test that cascaded definition is happening for non constants. */ > > > + > > > +void foo () > > > +{ > > > + extern int i; > > > + __vector int v,w; > > > + v = w = (vector int) { i }; > > > +} > > > + > > > +int main (int argc, char *argv[]) > > > +{ > > > + return 0; > > > +} > > > +/* { dg-final { scan-tree-dump-times " w = {i.0_1}" 1 "gimple" } } */ > > > > > > > > > >
On Fri, Oct 28, 2016 at 4:06 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > On Fri, 2016-10-28 at 08:31 -0500, Will Schmidt wrote: >> On Fri, 2016-10-28 at 10:38 +0200, Richard Biener wrote: >> > On Thu, Oct 27, 2016 at 5:37 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: >> > > Hi, >> > > >> > > Per PR72747, A statement such as "v = vec_splats (1);" correctly >> > > initializes a vector. However, a statement such as "v[1] = v[0] = >> > > vec_splats (1);" initializes both v[1] and v[0] to random garbage. >> > > >> > > It has been determined that this is occurring because we did not emit >> > > the actual initialization statement before our final exit from >> > > gimplify_init_constructor, at which time we lose the expression when we >> > > assign *expr_p to either NULL or object. This problem affected both constant >> > > and non-constant initializers. Corrected this by moving the logic to >> > > emit the statement up earlier within the if/else logic. >> > > >> > > Bootstrapped and make check ran without regressions on >> > > powerpc64le-unknown-linux-gnu. >> > > >> > > OK for trunk? >> > >> > Ok. >> > >> > RIchard. >> >> Committed revision 241647. >> Thanks. > > Oops, forgot to ask. After some burn-in time, is this OK to backport to > the 5 and 6 branches? Yes. Richard. > The bug was first noticed in GCC 5. > > Thanks, > -Will > >> >> >> > >> > > Thanks, >> > > -Will >> > > >> > > gcc: >> > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> >> > > >> > > PR middle-end/72747 >> > > * gimplify.c (gimplify_init_constructor): Move emit of constructor >> > > assignment to earlier in the if/else logic. >> > > >> > > testsuite: >> > > 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> >> > > >> > > PR middle-end/72747 >> > > * c-c++-common/pr72747-1.c: New test. >> > > * c-c++-common/pr72747-2.c: Likewise. >> > > >> > > Index: gcc/gimplify.c >> > > =================================================================== >> > > --- gcc/gimplify.c (revision 241600) >> > > +++ gcc/gimplify.c (working copy) >> > > @@ -4730,24 +4730,23 @@ >> > > >> > > if (ret == GS_ERROR) >> > > return GS_ERROR; >> > > - else if (want_value) >> > > + /* If we have gimplified both sides of the initializer but have >> > > + not emitted an assignment, do so now. */ >> > > + if (*expr_p) >> > > { >> > > + tree lhs = TREE_OPERAND (*expr_p, 0); >> > > + tree rhs = TREE_OPERAND (*expr_p, 1); >> > > + gassign *init = gimple_build_assign (lhs, rhs); >> > > + gimplify_seq_add_stmt (pre_p, init); >> > > + } >> > > + if (want_value) >> > > + { >> > > *expr_p = object; >> > > return GS_OK; >> > > } >> > > else >> > > { >> > > - /* If we have gimplified both sides of the initializer but have >> > > - not emitted an assignment, do so now. */ >> > > - if (*expr_p) >> > > - { >> > > - tree lhs = TREE_OPERAND (*expr_p, 0); >> > > - tree rhs = TREE_OPERAND (*expr_p, 1); >> > > - gassign *init = gimple_build_assign (lhs, rhs); >> > > - gimplify_seq_add_stmt (pre_p, init); >> > > - *expr_p = NULL; >> > > - } >> > > - >> > > + *expr_p = NULL; >> > > return GS_ALL_DONE; >> > > } >> > > } >> > > Index: gcc/testsuite/c-c++-common/pr72747-1.c >> > > =================================================================== >> > > --- gcc/testsuite/c-c++-common/pr72747-1.c (revision 0) >> > > +++ gcc/testsuite/c-c++-common/pr72747-1.c (working copy) >> > > @@ -0,0 +1,16 @@ >> > > +/* { dg-do compile } */ >> > > +/* { dg-require-effective-target powerpc_altivec_ok } */ >> > > +/* { dg-options "-maltivec -fdump-tree-gimple" } */ >> > > + >> > > +/* PR 72747: Test that cascaded definition is happening for constant vectors. */ >> > > + >> > > +#include <altivec.h> >> > > + >> > > +int main (int argc, char *argv[]) >> > > +{ >> > > + __vector int v1,v2; >> > > + v1 = v2 = vec_splats ((int) 42); >> > > + return 0; >> > > +} >> > > +/* { dg-final { scan-tree-dump-times " v2 = { 42, 42, 42, 42 }" 1 "gimple" } } */ >> > > + >> > > Index: gcc/testsuite/c-c++-common/pr72747-2.c >> > > =================================================================== >> > > --- gcc/testsuite/c-c++-common/pr72747-2.c (revision 0) >> > > +++ gcc/testsuite/c-c++-common/pr72747-2.c (working copy) >> > > @@ -0,0 +1,18 @@ >> > > +/* { dg-do compile } */ >> > > +/* { dg-require-effective-target powerpc_altivec_ok } */ >> > > +/* { dg-options "-c -maltivec -fdump-tree-gimple" } */ >> > > + >> > > +/* PR 72747: test that cascaded definition is happening for non constants. */ >> > > + >> > > +void foo () >> > > +{ >> > > + extern int i; >> > > + __vector int v,w; >> > > + v = w = (vector int) { i }; >> > > +} >> > > + >> > > +int main (int argc, char *argv[]) >> > > +{ >> > > + return 0; >> > > +} >> > > +/* { dg-final { scan-tree-dump-times " w = {i.0_1}" 1 "gimple" } } */ >> > > >> > > >> > >> >> > >
Index: gcc/gimplify.c =================================================================== --- gcc/gimplify.c (revision 241600) +++ gcc/gimplify.c (working copy) @@ -4730,24 +4730,23 @@ if (ret == GS_ERROR) return GS_ERROR; - else if (want_value) + /* If we have gimplified both sides of the initializer but have + not emitted an assignment, do so now. */ + if (*expr_p) { + tree lhs = TREE_OPERAND (*expr_p, 0); + tree rhs = TREE_OPERAND (*expr_p, 1); + gassign *init = gimple_build_assign (lhs, rhs); + gimplify_seq_add_stmt (pre_p, init); + } + if (want_value) + { *expr_p = object; return GS_OK; } else { - /* If we have gimplified both sides of the initializer but have - not emitted an assignment, do so now. */ - if (*expr_p) - { - tree lhs = TREE_OPERAND (*expr_p, 0); - tree rhs = TREE_OPERAND (*expr_p, 1); - gassign *init = gimple_build_assign (lhs, rhs); - gimplify_seq_add_stmt (pre_p, init); - *expr_p = NULL; - } - + *expr_p = NULL; return GS_ALL_DONE; } } Index: gcc/testsuite/c-c++-common/pr72747-1.c =================================================================== --- gcc/testsuite/c-c++-common/pr72747-1.c (revision 0) +++ gcc/testsuite/c-c++-common/pr72747-1.c (working copy) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-options "-maltivec -fdump-tree-gimple" } */ + +/* PR 72747: Test that cascaded definition is happening for constant vectors. */ + +#include <altivec.h> + +int main (int argc, char *argv[]) +{ + __vector int v1,v2; + v1 = v2 = vec_splats ((int) 42); + return 0; +} +/* { dg-final { scan-tree-dump-times " v2 = { 42, 42, 42, 42 }" 1 "gimple" } } */ + Index: gcc/testsuite/c-c++-common/pr72747-2.c =================================================================== --- gcc/testsuite/c-c++-common/pr72747-2.c (revision 0) +++ gcc/testsuite/c-c++-common/pr72747-2.c (working copy) @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-options "-c -maltivec -fdump-tree-gimple" } */ + +/* PR 72747: test that cascaded definition is happening for non constants. */ + +void foo () +{ + extern int i; + __vector int v,w; + v = w = (vector int) { i }; +} + +int main (int argc, char *argv[]) +{ + return 0; +} +/* { dg-final { scan-tree-dump-times " w = {i.0_1}" 1 "gimple" } } */
Hi, Per PR72747, A statement such as "v = vec_splats (1);" correctly initializes a vector. However, a statement such as "v[1] = v[0] = vec_splats (1);" initializes both v[1] and v[0] to random garbage. It has been determined that this is occurring because we did not emit the actual initialization statement before our final exit from gimplify_init_constructor, at which time we lose the expression when we assign *expr_p to either NULL or object. This problem affected both constant and non-constant initializers. Corrected this by moving the logic to emit the statement up earlier within the if/else logic. Bootstrapped and make check ran without regressions on powerpc64le-unknown-linux-gnu. OK for trunk? Thanks, -Will gcc: 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> PR middle-end/72747 * gimplify.c (gimplify_init_constructor): Move emit of constructor assignment to earlier in the if/else logic. testsuite: 2016-10-26 Will Schmidt <will_schmidt@vnet.ibm.com> PR middle-end/72747 * c-c++-common/pr72747-1.c: New test. * c-c++-common/pr72747-2.c: Likewise.