This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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
> <wschmidt@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
> >>
> >
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]