On Fri, 2018-04-13 at 16:54 -0500, Segher Boessenkool wrote:
> Hi Carl,
>
> On Fri, Apr 13, 2018 at 09:49:25AM -0700, Carl Love wrote:
> > diff --git a/gcc/config/rs6000/rs6000.c
> > b/gcc/config/rs6000/rs6000.c
> > index a0c9b5c..855be43 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -16576,8 +16576,9 @@ rs6000_gimple_fold_builtin
> > (gimple_stmt_iterator *gsi)
> > {
> > arg0 = gimple_call_arg (stmt, 0);
> > lhs = gimple_call_lhs (stmt);
> > - /* Only fold the vec_splat_*() if arg0 is constant. */
> > - if (TREE_CODE (arg0) != INTEGER_CST)
> > + /* Only fold the vec_splat_*() if arg0 is a 5-bit
> > constant. */
> > + if (TREE_CODE (arg0) != INTEGER_CST
> > + || TREE_INT_CST_LOW (arg0) & ~0x1f)
> > return false;
>
> Should this test for *signed* 5-bit constants only?
>
> if (TREE_CODE (arg0) != INTEGER_CST
> || !IN_RANGE (TREE_INT_CST_LOW (arg0), -16, 15))
Segher:
The buitins for the unsigned vec_splat_u[8, 16,32] are actually mapped
to their corresponding signed version vec_splat_s[8, 16,32] in
altivec.h. Both the vec_splat_u[8, 16,32] and vec_splat_s[8, 16,32]
builtins will get you to the case statement
/* flavors of vec_splat_[us]{8,16,32}. */
case ALTIVEC_BUILTIN_VSPLTISB:
case ALTIVEC_BUILTIN_VSPLTISH:
case ALTIVEC_BUILTIN_VSPLTISW:
under which the change is being made. So technically arg0 could be a
signed 5-bit or an unsiged 5-bit value. Either way the 5-bit value is
splatted into the vector with sign extension to 8/16/32 bits. So I
would argue that the IN_RANGE test for signed values is maybe
misleading as arg0 could represent a signed or unsigned value. Both
tests, the one from the patch or Segher's suggestion, are really just
looking to see if any of the upper bits are 1. From a functional
standpoint, I don't see any difference and feel either one would do the
job. Am I missing anything?
Carl Love
