| Message ID | BANLkTi=zPnDtoXNgD8S3RqtPqAYgZYTYmw@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Jun 10, 2011 at 9:19 AM, Ira Rosen <ira.rosen@linaro.org> wrote: > Hi, > > The test in PR 49318 fails because the vectorizer recognizes address > computation sequence as a widening-multiplication pattern, while such > sequence is not relevant to vectorization. The problem is that the > vectorizer doesn't check if a statement is going to be vectorized > before replacing it with a pattern. Moreover, the vectorizer first > detects the patterns and only after that looks for relevant > statements. Changing the order is not a good option, since statements > relevance is defined also by their belonging to a pattern. > > This patch solves the problem by removing pattern statements that were > created for statements that are not supposed to be vectorized. > > Bootstrapped with vectorization enabled on powerpc64-suse-linux and > tested on powerpc64-suse-linux and x86_64-suse-linux. > Committed. Ick, yeah. I remember running into this ordering issue when doing the multi-vector size reorgs... In the end I think we should not generate the pattern stmt during pattern matching but only mark the relevant statements with a pattern kind. Say, for each pattern we have a "main" statement that has related stmts belonging to the pattern that define uses of the "main" statement - mark those to refer to that "main" statement. For that "main" statement simply record an enum value, like, widening_mult. Then only at vectorized statement generation time actually generate the vectorized form of the pattern statement. Thus, the non-vectorized pattern statements would never be generated. So, separate analysis and transform more properly. Of course I didn't get around to implement the above ... (we'd have a new vectorizable_pattern worker). Richard. > Ira > > ChangeLog: > > PR tree-optimization/49318 > * tree-vect-loop.c (vect_determine_vectorization_factor): > Remove irrelevant pattern statements. > > testsuite/ChangeLog: > > PR tree-optimization/49318 > * gcc.dg/vect/pr49318.c: New test. >
On 10 June 2011 12:14, Richard Guenther <richard.guenther@gmail.com> wrote: > On Fri, Jun 10, 2011 at 9:19 AM, Ira Rosen <ira.rosen@linaro.org> wrote: >> Hi, >> >> The test in PR 49318 fails because the vectorizer recognizes address >> computation sequence as a widening-multiplication pattern, while such >> sequence is not relevant to vectorization. The problem is that the >> vectorizer doesn't check if a statement is going to be vectorized >> before replacing it with a pattern. Moreover, the vectorizer first >> detects the patterns and only after that looks for relevant >> statements. Changing the order is not a good option, since statements >> relevance is defined also by their belonging to a pattern. >> >> This patch solves the problem by removing pattern statements that were >> created for statements that are not supposed to be vectorized. >> >> Bootstrapped with vectorization enabled on powerpc64-suse-linux and >> tested on powerpc64-suse-linux and x86_64-suse-linux. >> Committed. > > Ick, yeah. I remember running into this ordering issue when doing the > multi-vector size reorgs... > > In the end I think we should not generate the pattern stmt during > pattern matching but only mark the relevant statements with a > pattern kind. Say, for each pattern we have a "main" statement > that has related stmts belonging to the pattern that define uses > of the "main" statement - mark those to refer to that "main" statement. > For that "main" statement simply record an enum value, like, > widening_mult. Then only at vectorized statement > generation time actually generate the vectorized form of the > pattern statement. > > Thus, the non-vectorized pattern statements would never be generated. > > So, separate analysis and transform more properly. > > Of course I didn't get around to implement the above ... (we'd have > a new vectorizable_pattern worker). Sounds like a good idea. I'll give it a try. Thanks, Ira > > Richard. > >> Ira >> >> ChangeLog: >> >> PR tree-optimization/49318 >> * tree-vect-loop.c (vect_determine_vectorization_factor): >> Remove irrelevant pattern statements. >> >> testsuite/ChangeLog: >> >> PR tree-optimization/49318 >> * gcc.dg/vect/pr49318.c: New test. >> >
Index: ChangeLog =================================================================== --- ChangeLog (revision 174889) +++ ChangeLog (working copy) @@ -1,3 +1,9 @@ +2011-06-10 Ira Rosen <ira.rosen@linaro.org> + + PR tree-optimization/49318 + * tree-vect-loop.c (vect_determine_vectorization_factor): Remove + irrelevant pattern statements. + 2011-06-10 Hans-Peter Nilsson <hp@axis.com> * system.h (SETJMP_VIA_SAVE_AREA): Poison. Index: testsuite/gcc.dg/vect/pr49318.c =================================================================== --- testsuite/gcc.dg/vect/pr49318.c (revision 0) +++ testsuite/gcc.dg/vect/pr49318.c (revision 0) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target vect_int } */ +/* { dg-require-effective-target vect_float } */ + +typedef enum { GL_FALSE } GLenum; +typedef unsigned char GLboolean; +typedef int GLint; +typedef unsigned int GLuint; +typedef float GLfloat; +typedef double GLdouble; +typedef struct gl_context GLcontext; +struct gl_context { + GLfloat TextureMatrix[16]; + GLenum Primitive; +}; +void gl_GetDoublev( GLcontext *ctx, GLenum pname, GLdouble *params ) { + GLuint i; + for (i=0; i<16; i++) + params[i] = (GLint) ctx->TextureMatrix[i]; +} + +/* { dg-final { cleanup-tree-dump "vect" } } */ Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 174889) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2011-06-10 Ira Rosen <ira.rosen@linaro.org> + + PR tree-optimization/49318 + * gcc.dg/vect/pr49318.c: New test. + 2011-06-09 David Krauss <potswa@mac.com> * g++.dg/template/arrow1.C: New. Index: tree-vect-loop.c =================================================================== --- tree-vect-loop.c (revision 174889) +++ tree-vect-loop.c (working copy) @@ -255,10 +255,20 @@ vect_determine_vectorization_factor (loop_vec_info gcc_assert (stmt_info); - /* skip stmts which do not need to be vectorized. */ + /* Skip stmts which do not need to be vectorized. */ if (!STMT_VINFO_RELEVANT_P (stmt_info) && !STMT_VINFO_LIVE_P (stmt_info)) { + if (is_pattern_stmt_p (stmt_info)) + { + /* We are not going to vectorize this pattern statement, + therefore, remove it. */ + gimple_stmt_iterator tmp_gsi = gsi_for_stmt (stmt); + STMT_VINFO_RELATED_STMT (stmt_info) = NULL; + gsi_remove (&tmp_gsi, true); + free_stmt_vec_info (stmt); + } + if (vect_print_dump_info (REPORT_DETAILS)) fprintf (vect_dump, "skip."); continue;
Hi, The test in PR 49318 fails because the vectorizer recognizes address computation sequence as a widening-multiplication pattern, while such sequence is not relevant to vectorization. The problem is that the vectorizer doesn't check if a statement is going to be vectorized before replacing it with a pattern. Moreover, the vectorizer first detects the patterns and only after that looks for relevant statements. Changing the order is not a good option, since statements relevance is defined also by their belonging to a pattern. This patch solves the problem by removing pattern statements that were created for statements that are not supposed to be vectorized. Bootstrapped with vectorization enabled on powerpc64-suse-linux and tested on powerpc64-suse-linux and x86_64-suse-linux. Committed. Ira ChangeLog: PR tree-optimization/49318 * tree-vect-loop.c (vect_determine_vectorization_factor): Remove irrelevant pattern statements. testsuite/ChangeLog: PR tree-optimization/49318 * gcc.dg/vect/pr49318.c: New test.