[Bug tree-optimization/101801] New: vect_worthwhile_without_simd_p is broken

[Bug tree-optimization/101801] New: vect_worthwhile_without_simd_p is broken

[rguenth at gcc dot gnu.org]

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

            Bug ID: 101801
           Summary: vect_worthwhile_without_simd_p is broken
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

vect_worthwhile_without_simd_p is currently

bool
vect_worthwhile_without_simd_p (vec_info *vinfo, tree_code code)
{
  loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo);
  unsigned HOST_WIDE_INT value;
  return (loop_vinfo
          && LOOP_VINFO_VECT_FACTOR (loop_vinfo).is_constant (&value)
          && value >= vect_min_worthwhile_factor (code));
}

which means it's never worthwhile to BB vectorize.  Also the VF check
doesn't honor SLP so that a fully SLPed loop with VF == 1 is never
considered worthwhile to vectorize.

I ran into this beast when looking at vectorization of mask condition
operations like cond_mask1 & cond_mask2 which, for AVX512, have
integer mode but vectorizable_operation does

  /* Worthwhile without SIMD support?  Check only during analysis.  */
  if (!VECTOR_MODE_P (vec_mode)
      && !vec_stmt
      && !vect_worthwhile_without_simd_p (vinfo, code))
    {
      if (dump_enabled_p ())
        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
                         "not worthwhile without SIMD support.\n");
      return false;
    }

and in my case with SLP the VF was indeed one and vectorization failed.
I think the code should not look at the vectorization factor but instead
at the vector type (and its number of components).

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org
             Blocks|                            |53947
           Keywords|                            |missed-optimization

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'm not sure why we have vect_worthwhile_without_simd_p at all (it looks like a
cost thing and thus should be an overall assessment and not local spread across
vectorizable_*).

Any objection to kill it off completely?


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53947
[Bug 53947] [meta-bug] vectorizer missed-optimizations

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[rsandifo at gcc dot gnu.org]

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

--- Comment #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Never really looked at the SIMD-without-SIMD stuff.
When I first saw this, I was hoping you were suggesting
killing off the whole thing :-)

So no, no objection from me.  It sounds like in the motivating
case we should really be vectorising the mask operations as
vectors of booleans though, even if the vectors have an
integer TYPE_MODE.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[rsandifo at gcc dot gnu.org]

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

--- Comment #3 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
So maybe a less invasive fix would be to add && !VECTOR_BOOLEAN_TYPE_P
to the condition.  Still no objection to killing it off instead though.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #3)
> So maybe a less invasive fix would be to add && !VECTOR_BOOLEAN_TYPE_P
> to the condition.  Still no objection to killing it off instead though.

Yeah, I've pondered adding && !mask_op_p - but I'm not sure if
VECTOR_BOOLEAN_TYPE_P can be generic vectors (I guess not at the moment).
So detecting what is a generic vector and what not seems fragile.

Btw, we _do_ vectorize using vector booleans, it's just the check
for vect_worthwhile_without_simd_p misclassifies the non-vector-mode
types as "generic".

I'm currently checking whether there's any testsuite fallout in making
vect_worthwhile_without_simd_p return true unconditionally.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:f31da42e047e8018ca6ad9809273bc7efb6ffcaf

commit r12-2789-gf31da42e047e8018ca6ad9809273bc7efb6ffcaf
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Aug 6 14:39:05 2021 +0200

    tree-optimization/101801 - remove vect_worthwhile_without_simd_p

    This removes the cost part of vect_worthwhile_without_simd_p, retaining
    only the correctness bits.  The reason is that the cost heuristic
    do not properly account for SLP plus the check whether "without simd"
    applies misfires for AVX512 mask vectors at the moment, leading to
    missed vectorizations there.

    Any costing decision should take place in the cost modeling, no
    single stmt is to disable all vectorization on its own.

    2021-08-06  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/101801
            * tree-vectorizer.h (vect_worthwhile_without_simd_p): Rename...
            (vect_can_vectorize_without_simd_p): ... to this.
            * tree-vect-loop.c (vect_worthwhile_without_simd_p): Rename...
            (vect_can_vectorize_without_simd_p): ... to this and fold
            in vect_min_worthwhile_factor.
            (vect_min_worthwhile_factor): Remove.
            (vectorizable_reduction): Adjust and remove the cost part.
            * tree-vect-stmts.c (vectorizable_shift): Likewise.
            (vectorizable_operation): Likewise.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:19d1a529fa9f78e7ec7be38d423c90e00cec8f8c

commit r12-2832-g19d1a529fa9f78e7ec7be38d423c90e00cec8f8c
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 9 11:42:47 2021 +0200

    tree-optimization/101801 - rework generic vector vectorization more

    This builds ontop of the vect_worthwhile_without_simd_p refactoring
    done earlier.  It was wrong in dropping the appearant double checks
    for operation support since the optab check can happen with an
    integer vector emulation mode and thus succeed but vector lowering
    might not actually support the operation on word_mode.

    The following patch adds a vect_emulated_vector_p helper and
    re-instantiates the check where it was previously.  It also adds
    appropriate costing of the scalar stmts emitted by vector lowering
    to vectorizable_operation which should be the only place such
    operations are synthesized.  I've also cared for the case where
    the vector mode is supported but the operation is not (though
    I think this will be unlikely given we're talking about plus, minus
    and negate).

    This fixes the observed FAIL of gcc.dg/tree-ssa/gen-vect-11b.c
    with -m32 where we end up vectorizing a multiplication that ends up
    being teared down to scalars again by vector lowering.

    I'm not super happy about all the other places where we're now
    and previously feeding scalar modes to optab checks where we
    want to know whether we can vectorize sth but well.

    2021-09-08  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/101801
            PR tree-optimization/101819
            * tree-vectorizer.h (vect_emulated_vector_p): Declare.
            * tree-vect-loop.c (vect_emulated_vector_p): New function.
            (vectorizable_reduction): Re-instantiate a check for emulated
            operations.
            * tree-vect-stmts.c (vectorizable_shift): Likewise.
            (vectorizable_operation): Likewise.  Cost emulated vector
            operations according to the scalar sequence synthesized by
            vector lowering.

[Bug tree-optimization/101801] vect_worthwhile_without_simd_p is broken

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0