This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix PR50031, take 2
On Fri, 2012-02-10 at 16:22 +0100, Richard Guenther wrote:
> On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt
> <email@example.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.
> > 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