Re: [PATCH 2/3] Refactor widen_plus as internal_fn

[Richard Sandiford <richard.sandiford@arm.com>]

Just some very minor things.

"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 5c9da73ea11f8060b18dcf513599c9694fa4f2ad..348bee35a35ae4ed9a8652f5349f430c2733e1cb 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -90,6 +90,71 @@ lookup_internal_fn (const char *name)
>    return entry ? *entry : IFN_LAST;
>  }
>  
> +/*  Given an internal_fn IFN that is either a widening or narrowing function, return its
> +    corresponding LO and HI internal_fns.  */

Long line and too much space after "/*":

/* Given an internal_fn IFN that is either a widening or narrowing function,
   return its corresponding _LO and _HI internal_fns in *LO and *HI.  */

> +extern void
> +lookup_hilo_internal_fn (internal_fn ifn, internal_fn *lo, internal_fn *hi)
> +{
> +  gcc_assert (widening_fn_p (ifn) || narrowing_fn_p (ifn));
> +
> +  switch (ifn)
> +    {
> +    default:
> +      gcc_unreachable ();
> +#undef DEF_INTERNAL_FN
> +#undef DEF_INTERNAL_WIDENING_OPTAB_FN
> +#undef DEF_INTERNAL_NARROWING_OPTAB_FN
> +#define DEF_INTERNAL_FN(NAME, FLAGS, TYPE)
> +#define DEF_INTERNAL_WIDENING_OPTAB_FN(NAME, F, S, SO, UO, T)	\
> +    case IFN_##NAME:						\
> +      *lo = internal_fn (IFN_##NAME##_LO);			\
> +      *hi = internal_fn (IFN_##NAME##_HI);			\
> +      break;
> +#define DEF_INTERNAL_NARROWING_OPTAB_FN(NAME, F, O, T)	\
> +    case IFN_##NAME:					\
> +      *lo = internal_fn (IFN_##NAME##_LO);		\
> +      *hi = internal_fn (IFN_##NAME##_HI);		\
> +      break;
> +#include "internal-fn.def"
> +#undef DEF_INTERNAL_FN
> +#undef DEF_INTERNAL_WIDENING_OPTAB_FN
> +#undef DEF_INTERNAL_NARROWING_OPTAB_FN
> +    }
> +}
> +
> +extern void
> +lookup_evenodd_internal_fn (internal_fn ifn, internal_fn *even,
> +			    internal_fn *odd)

This needs a similar comment:

/* Given an internal_fn IFN that is either a widening or narrowing function,
   return its corresponding _EVEN and _ODD internal_fns in *EVEN and *ODD.  */

> @@ -3971,6 +4036,9 @@ commutative_binary_fn_p (internal_fn fn)
>      case IFN_UBSAN_CHECK_MUL:
>      case IFN_ADD_OVERFLOW:
>      case IFN_MUL_OVERFLOW:
> +    case IFN_VEC_WIDEN_PLUS:
> +    case IFN_VEC_WIDEN_PLUS_LO:
> +    case IFN_VEC_WIDEN_PLUS_HI:

Should include even & odd as well.

I'd suggest leaving out the narrowing stuff for now.  There are some
questions that would be easier to answer once we add the first use,
such as whether one of the hi/lo pair and one or the even/odd pair
merge with a vector containing the other half, whether all four
define the other half to be zero, etc.

OK for the optab/internal-fn parts with those changes from my POV.

Thanks again for doing this!

Richard