On Wed, Aug 29, 2018 at 11:25 AM Richard Sandiford <richard.sandif...@arm.com> wrote: > > Richard Biener <richard.guent...@gmail.com> writes: > > On Tue, Aug 28, 2018 at 1:25 PM Richard Sandiford > > <richard.sandif...@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
