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: [6/6] Link imm uses for pattern stmts


On Wed, Aug 29, 2018 at 11:25 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Tue, Aug 28, 2018 at 1:25 PM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> One of the warts of the vectoriser IR is that it doesn't link SSA name
> >> uses for pattern statements, leading to complicated special cases in
> >> vect_mark_stmts_to_be_vectorized and (especially) vect_detect_hybrid_slp.
> >> It also makes it harder to check how an SSA name is used after pattern
> >> replacement (something I need for a later patch).
> >>
> >> This patch adds a mode in which tree-ssa-operands.c can update
> >> statements in the same non-invasive way as for debug statements.
> >> It then uses this mode to update pattern statements when adding
> >> them to a vec_basic_block, so that pattern statements become
> >> even more like statements that existed from the outset.
> >
> > Without reviewing in detail yet I think putting them in the same
> > ballpark as debug stmts or documenting they have no effect on
> > code generation is misleading - this is because immediate use
> > handling _does_ affect code generation unless you mark those
> > stmts in some way and handle them like debug-uses in all of
> > the immediate use routines.  For example match-and-simplify
> > foldings guarded by :s will see the transparent stmts as uses.
>
> Yeah, true.  So the comment was plain wrong.  I guess what I meant to
> say was that adding the statement and removing it again must leave the
> IL the same as before.
>
> > So the real effect is to not add virtual operands or marking things
> > addressable?  IMHO rather than adding a transparent_p flag
> > we eventually should allow passing down a oep flag to update_stmt?
> >
> > Do vector pattern stmts cause any of the issues?  I don't remember
> > seeing things that would cause TREE_ADDRESSABLE.  I know
> > we have patterns for stores so we'd get extra VDEFs.
>
> Yeah, it was the vop case that motivated the patch.  I don't know of
> any specific examples in which pattern statements could cause the
> addressable flag to be set, but in principle we should handle that
> case like debug statements.
>
> The series was building up to adding support for extending loads and
> truncating stores, which would be another source of potential VUSEs and VDEFs.
>
> > IIRC we historically didn't add pattern stmts to the IL simply because
> > pattern recognition was supposed to be analysis phase and building
> > and eventually tearing down pattern stmts was thought as to be too
> > expensive.  That is also because we'd need to copy the loop to
> > retain a copy w/o those stmts.
> >
> > So - would update_stmt (stmt, opf_no_vops|opf_non_addressable)
> > work?  I suppose we'd need to pass down a second sticky_flags
> > arg.
>
> I don't mind making the opf_* flags part of the public interface
> if you think that's cleaner, but I'd still prefer to have a name
> that abstracts the semantics.  Using opf_no_vops|opf_non_addressable
> directly feels a bit low-level.

True.  I guess not exposing the opf_ flags would be better.  Maybe
really just clall the flag like_debug_stmt_p ;)  That makes the semantics
obvious as well - you can of course document that, for example, it
doesn't make things addressable or add virtual operands.

I suppose the patch is OK with that mere documentation change.

Richard.

>
> Thanks,
> Richard


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