This is the mail archive of the
mailing list for the GCC project.
Re: [6/6] Link imm uses for pattern stmts
On Wed, Aug 29, 2018 at 11:25 AM Richard Sandiford
> Richard Biener <email@example.com> writes:
> > On Tue, Aug 28, 2018 at 1:25 PM Richard Sandiford
> > <firstname.lastname@example.org> 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.