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, Feb 10, 2012 at 4:36 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
>
>
> 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.

Ok, looking at the code in tree-vect-patterns.c it seems that
STMT_VINFO_IN_PATTERN_P is only set for the stmt that contains
the first replacement stmt in STMT_VINFO_RELATED_STMT and further
stmts in that stmts STMT_VINFO_PATTERN_DEF_SEQ.  Other
stmts that were matched do not have STMT_VINFO_IN_PATTERN_P set
(but their STMT_VINFO_RELATED_STMT points to the stmt that has
STMT_VINFO_IN_PATTERN_P set).  Possibly the other scalar stmts
participating in the pattern are marked as not relevant (couldn't spot
this quickly).

So it seems that your patch should indeed work as-is.

Thus, ok, if Jakub doesn't spot another error.

Thanks,
Richard.

> ?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]