On Tue, Sep 2, 2025 at 11:55 PM Andrew Pinski
<[email protected]> wrote:
>
> On Tue, Sep 2, 2025 at 5:26 AM Richard Biener
> <[email protected]> wrote:
> >
> > On Sun, Aug 31, 2025 at 4:11 PM Andrew Pinski
> > <[email protected]> wrote:
> > >
> > > This is a small cleanup by moving the optimization of memcmp to
> > > memcmp_eq to fab from strlen pass. Since the copy of the other
> > > part of the memcmp strlen optimization to forwprop, this was the
> > > only thing left that strlen can do memcmp.
> > >
> > > Note this move will cause memcmp_eq to be used for -Os too.
> > >
> > > It also removes the optimization from strlen since both are now
> > > handled elsewhere.
> > >
> > > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > OK.
> >
> > Note I don't like fab much, at least the parts that are not obviously
> > instruction selection.  Most of it should be part of forwprop or ISEL
> > or a pre-expand "cleanup" (removal of __builtin_assume_aligned and friends).
>
> I was thinking about moving some of fab into optimize_widening_mul
> (which in itself is already misnamed) like the atomic builtins
> simplifications and the stdarg ones.
>
> >
> > So, unless memcmp_eq is unhandled elsewhere and thus we need to
> > do it late (-> ISEL), I'd rather do it in forwprop.
>
> memcmp_eq is pretty much not handled anywhere in the gimple. Now I
> think most of the places which do handle memcmp can/should handle
> memcmp_eq in a similar way.
> Now this is really an isel like what is done for popcount but that is
> done in optimize_widening_mul. Since both are similar in nature they
> should be in isel really since they explain how these 2 are used to
> expand and don't need anything more than that.
> I will file a few bugs about moving this stuff around and trying to
> remove some extra passes but I don't think I will get to them until
> next year.

That's fine with me and I agree that widen_mul is another ISEL.

Richard.

> Thanks,
> Andrew Pinski
>
>
>
> >
> > Richard.
> >
> > > gcc/ChangeLog:
> > >
> > >         * tree-ssa-ccp.cc (optimize_memcmp_eq): New function.
> > >         (pass_fold_builtins::execute): Call optimize_memcmp_eq
> > >         for memcmp.
> > >         * tree-ssa-strlen.cc (strlen_pass::handle_builtin_memcmp): Remove.
> > >         (strlen_pass::check_and_optimize_call): Don't call 
> > > handle_builtin_memcmp.
> > >
> > > Signed-off-by: Andrew Pinski <[email protected]>
> > > ---
> > >  gcc/tree-ssa-ccp.cc    | 35 +++++++++++++++++++++++
> > >  gcc/tree-ssa-strlen.cc | 63 ------------------------------------------
> > >  2 files changed, 35 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
> > > index f33cc042e9f..f16b37f193e 100644
> > > --- a/gcc/tree-ssa-ccp.cc
> > > +++ b/gcc/tree-ssa-ccp.cc
> > > @@ -155,6 +155,7 @@ along with GCC; see the file COPYING3.  If not see
> > >  #include "ipa-prop.h"
> > >  #include "internal-fn.h"
> > >  #include "gimple-range.h"
> > > +#include "tree-ssa-strlen.h"
> > >
> > >  /* Possible lattice values.  */
> > >  typedef enum
> > > @@ -4221,6 +4222,36 @@ public:
> > >
> > >  }; // class pass_fold_builtins
> > >
> > > +/* Optimize memcmp STMT into memcmp_eq if it is only used with
> > > +   `== 0` or `!= 0`. */
> > > +
> > > +static void
> > > +optimize_memcmp_eq (gcall *stmt)
> > > +{
> > > +  /* Make sure memcmp arguments are the correct type.  */
> > > +  if (gimple_call_num_args (stmt) != 3)
> > > +    return;
> > > +  tree arg1 = gimple_call_arg (stmt, 0);
> > > +  tree arg2 = gimple_call_arg (stmt, 1);
> > > +  tree len = gimple_call_arg (stmt, 2);
> > > +
> > > +  if (!POINTER_TYPE_P (TREE_TYPE (arg1)))
> > > +    return;
> > > +  if (!POINTER_TYPE_P (TREE_TYPE (arg2)))
> > > +    return;
> > > +  if (!INTEGRAL_TYPE_P (TREE_TYPE (len)))
> > > +    return;
> > > +  /* The return value of the memcmp has to be used
> > > +     equality comparison to zero. */
> > > +  tree res = gimple_call_lhs (stmt);
> > > +
> > > +  if (!res || !use_in_zero_equality (res))
> > > +    return;
> > > +
> > > +  gimple_call_set_fndecl (stmt, builtin_decl_explicit 
> > > (BUILT_IN_MEMCMP_EQ));
> > > +  update_stmt (stmt);
> > > +}
> > > +
> > >  unsigned int
> > >  pass_fold_builtins::execute (function *fun)
> > >  {
> > > @@ -4285,6 +4316,10 @@ pass_fold_builtins::execute (function *fun)
> > >                   gsi_next (&i);
> > >                   continue;
> > >
> > > +               case BUILT_IN_MEMCMP:
> > > +                 optimize_memcmp_eq (as_a<gcall*>(stmt));
> > > +                 break;
> > > +
> > >                 case BUILT_IN_UNREACHABLE:
> > >                   if (optimize_unreachable (i))
> > >                     cfg_changed = true;
> > > diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc
> > > index c4d64132e53..ade8c7dcadd 100644
> > > --- a/gcc/tree-ssa-strlen.cc
> > > +++ b/gcc/tree-ssa-strlen.cc
> > > @@ -3971,65 +3971,6 @@ use_in_zero_equality (tree res, bool exclusive)
> > >    return first_use;
> > >  }
> > >
> > > -/* Handle a call to memcmp.  We try to handle small comparisons by
> > > -   converting them to load and compare, and replacing the call to memcmp
> > > -   with a __builtin_memcmp_eq call where possible.
> > > -   return true when call is transformed, return false otherwise.  */
> > > -
> > > -bool
> > > -strlen_pass::handle_builtin_memcmp ()
> > > -{
> > > -  gcall *stmt = as_a <gcall *> (gsi_stmt (m_gsi));
> > > -  tree res = gimple_call_lhs (stmt);
> > > -
> > > -  if (!res || !use_in_zero_equality (res))
> > > -    return false;
> > > -
> > > -  tree arg1 = gimple_call_arg (stmt, 0);
> > > -  tree arg2 = gimple_call_arg (stmt, 1);
> > > -  tree len = gimple_call_arg (stmt, 2);
> > > -  unsigned HOST_WIDE_INT leni;
> > > -
> > > -  if (tree_fits_uhwi_p (len)
> > > -      && (leni = tree_to_uhwi (len)) <= GET_MODE_SIZE (word_mode)
> > > -      && pow2p_hwi (leni))
> > > -    {
> > > -      leni *= CHAR_TYPE_SIZE;
> > > -      unsigned align1 = get_pointer_alignment (arg1);
> > > -      unsigned align2 = get_pointer_alignment (arg2);
> > > -      unsigned align = MIN (align1, align2);
> > > -      scalar_int_mode mode;
> > > -      if (int_mode_for_size (leni, 1).exists (&mode)
> > > -         && (align >= leni || !targetm.slow_unaligned_access (mode, 
> > > align)))
> > > -       {
> > > -         location_t loc = gimple_location (stmt);
> > > -         tree type, off;
> > > -         type = build_nonstandard_integer_type (leni, 1);
> > > -         gcc_assert (known_eq (GET_MODE_BITSIZE (TYPE_MODE (type)), 
> > > leni));
> > > -         tree ptrtype = build_pointer_type_for_mode (char_type_node,
> > > -                                                     ptr_mode, true);
> > > -         off = build_int_cst (ptrtype, 0);
> > > -         arg1 = build2_loc (loc, MEM_REF, type, arg1, off);
> > > -         arg2 = build2_loc (loc, MEM_REF, type, arg2, off);
> > > -         tree tem1 = fold_const_aggregate_ref (arg1);
> > > -         if (tem1)
> > > -           arg1 = tem1;
> > > -         tree tem2 = fold_const_aggregate_ref (arg2);
> > > -         if (tem2)
> > > -           arg2 = tem2;
> > > -         res = fold_convert_loc (loc, TREE_TYPE (res),
> > > -                                 fold_build2_loc (loc, NE_EXPR,
> > > -                                                  boolean_type_node,
> > > -                                                  arg1, arg2));
> > > -         gimplify_and_update_call_from_tree (&m_gsi, res);
> > > -         return true;
> > > -       }
> > > -    }
> > > -
> > > -  gimple_call_set_fndecl (stmt, builtin_decl_explicit 
> > > (BUILT_IN_MEMCMP_EQ));
> > > -  return true;
> > > -}
> > > -
> > >  /* Given strinfo IDX for ARG, sets LENRNG[] to the range of lengths
> > >     of the string(s) referenced by ARG if it can be determined.
> > >     If the length cannot be determined, sets *SIZE to the size of
> > > @@ -5520,10 +5461,6 @@ strlen_pass::check_and_optimize_call (bool 
> > > *zero_write)
> > >        if (handle_builtin_memset (zero_write))
> > >         return false;
> > >        break;
> > > -    case BUILT_IN_MEMCMP:
> > > -      if (handle_builtin_memcmp ())
> > > -       return false;
> > > -      break;
> > >      case BUILT_IN_STRCMP:
> > >      case BUILT_IN_STRNCMP:
> > >        if (handle_builtin_string_cmp ())
> > > --
> > > 2.43.0
> > >

Reply via email to