On Fri, 2012-02-10 at 16:22 +0100, Richard Guenther wrote: > On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt > <wschm...@linux.vnet.ibm.com> wrote: > > Jakub, thanks! Based on this, I believe the patch is correct in its > > handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double > > counting. > > > > I misinterpreted what the commentary for vect_pattern_recog was saying: > > I thought that all replacements were hung off of the last pattern > > statement, but this was just true for the particular example, where only > > one replacement statement was created. Sorry for any confusion! > > > > Based on the discussion, is the patch OK for trunk? > > But you still count for each of the matched scalar stmts the cost of the > whole pattern sequence. No?
I need to change the commentary to make this more clear now. My comment: "Translate the last statement..." is incorrect; this is done for each statement in a pattern. Per Jakub's explanation, the replacement statements are distributed over the original pattern statements. Visiting STMT_VINFO_RELATED_STMT for a statement marked STMT_VINFO_IN_PATTERN_P will find zero or one replacement statements that should be examined. Visiting STMT_VINFO_PATTERN_DEF_SEQ may pick up some leftover replacement statements that didn't fit in the 1-1 mapping. The point is that all of these related statements and pattern-def-seq statements are disjoint, and Ira's logic is ensuring that they all get examined once. It's not a very clean way to represent the replacement of a pattern -- not how you'd do it if designing from scratch -- but I guess I can see how it got this way when the STMT_VINFO_PATTERN_DEF_SEQ was glued onto the existing 1-1 mapping. Bill > > Richard. > > > Thanks, > > Bill > > > > On Fri, 2012-02-10 at 14:44 +0100, Jakub Jelinek wrote: > >> On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote: > >> > >From the commentary at the end of tree-vect-patterns.c, only the main > >> > statement in the pattern (the last one) is flagged as > >> > STMT_VINFO_IN_PATTERN_P. So this is finding the new replacement > >> > statement which has been created but not yet inserted in the code. It > >> > only gets counted once. > >> > >> STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not > >> just the last, but all original stmts that are being replaced by the > >> pattern). Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt > >> (last one corresponding to that original stmt). If needed, further > >> pattern stmts for that original stmts are put into > >> STMT_VINFO_PATTERN_DEF_SEQ. The pattern matcher could e.g. match > >> 3 original stmts, and stick a single pattern stmt into > >> STMT_VINFO_RELATED_STMT > >> on the first original stmt, then say 2 pattern stmts into > >> STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one > >> STMT_VINFO_RELATED_STMT and finally one pattern stmt through > >> STMT_VINFO_RELATED_STMT on the third original stmt. > >> > >> > Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so > >> > this section has to be omitted if we backport it (which is desirable > >> > since the degradation was introduced in 4.6). Removing it apparently > >> > does not affect the sphinx3 degradation. > >> > >> Yeah, when backporting you can basically assume that > >> STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL > >> instead > >> of itt (and simplify), because no pattern recognizers needed more than one > >> pattern stmt for each original stmt back then. > >> > >> Jakub > >> > > >
