[Bug tree-optimization/113091] Over-estimate SLP vector-to-scalar cost for non-live pattern statement

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113091

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's the logic

  FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt_info)
    {
      if (svisited.contains (stmt_info))
        continue;
      stmt_vec_info orig_stmt_info = vect_orig_stmt (stmt_info);
      if (STMT_VINFO_IN_PATTERN_P (orig_stmt_info)
          && STMT_VINFO_RELATED_STMT (orig_stmt_info) != stmt_info)
        /* Only the pattern root stmt computes the original scalar value.  */
        continue;
      bool mark_visited = true;
      gimple *orig_stmt = orig_stmt_info->stmt;
      ssa_op_iter op_iter;
      def_operand_p def_p;
      FOR_EACH_PHI_OR_STMT_DEF (def_p, orig_stmt, op_iter, SSA_OP_DEF)
        {
          imm_use_iterator use_iter;
          gimple *use_stmt;
          stmt_vec_info use_stmt_info;
          FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, DEF_FROM_PTR (def_p))
            if (!is_gimple_debug (use_stmt))
              {
                use_stmt_info = bb_vinfo->lookup_stmt (use_stmt);
                if (!use_stmt_info
                    || !PURE_SLP_STMT (vect_stmt_to_vectorize (use_stmt_info)))
                  {
                    STMT_VINFO_LIVE_P (stmt_info) = true;

specifically the last check.  That's supposed to pick up the "main" pattern
that's now covering the scalar stmt.

But somehow the "main" pattern,

patt_67 = .VEC_WIDEN_MINUS (_1, _3);  //  _5 = _2 - _4;
 patt_68 = (signed short) patt_67;     //  _5 = _2 - _4;
 patt_69 = (int) patt_68;              //  _5 = _2 - _4;

doesn't get picked up here.  I wonder what's the orig_stmt and the def
picked and what original scalar use we end up in where the
vect_stmt_to_vectorize isn't the "last" pattern.  Maybe we really want
these "overlapping" patterns, but IMHO having "two entries" into
a chain of scalar stmts is bad and we should link up the whole matched
sequence to the final "root" instead?

That said, the current code doesn't see that wherever we end up isn't
dead code (aka fully covered by the vectorization).

IMO vect_stmt_to_vectorize for each of those stmts should end up at

patt_69 = (int) patt_68;