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.

Thanks,
Richard

Reply via email to