On Mon, Aug 28, 2023 at 11:33:37AM +0200, Andreas Krebbel wrote:
> Hi Stefan,
>
> do you really need to introduce a new flag for U64 given that the type of the
> builtin is unsigned long?
In function s390_const_operand_ok the immediate is checked whether it is
valide w.r.t. the flag:
tree_to_uhwi (arg) > ((HOST_WIDE_INT_1U << (bitwidth - 1) << 1) - 1)
Here bitwidth is derived from the flag.
Cheers,
Stefan
>
> Andreas
>
> On 8/21/23 17:56, Stefan Schulze Frielinghaus wrote:
> > The second argument of these builtins is an unsigned immediate. For
> > vec_rli the API allows immediates up to 64 bits whereas the instruction
> > verll only allows immediates up to 32 bits. Since the shift count
> > equals the immediate modulo vector element size, truncating those
> > immediates is fine.
> >
> > Bootstrapped and regtested on s390. Ok for mainline?
> >
> > gcc/ChangeLog:
> >
> > * config/s390/s390-builtins.def (O_U64): New.
> > (O1_U64): Ditto.
> > (O2_U64): Ditto.
> > (O3_U64): Ditto.
> > (O4_U64): Ditto.
> > (O_M12): Change bit position.
> > (O_S2): Ditto.
> > (O_S3): Ditto.
> > (O_S4): Ditto.
> > (O_S5): Ditto.
> > (O_S8): Ditto.
> > (O_S12): Ditto.
> > (O_S16): Ditto.
> > (O_S32): Ditto.
> > (O_ELEM): Ditto.
> > (O_LIT): Ditto.
> > (OB_DEF_VAR): Add operand constraints.
> > (B_DEF): Ditto.
> > * config/s390/s390.cc (s390_const_operand_ok): Honour 64 bit
> > operands.
> > ---
> > gcc/config/s390/s390-builtins.def | 60 ++++++++++++++++++-------------
> > gcc/config/s390/s390.cc | 6 ++--
> > 2 files changed, 39 insertions(+), 27 deletions(-)
> >
> > diff --git a/gcc/config/s390/s390-builtins.def
> > b/gcc/config/s390/s390-builtins.def
> > index a16983b18bd..c829f445a11 100644
> > --- a/gcc/config/s390/s390-builtins.def
> > +++ b/gcc/config/s390/s390-builtins.def
> > @@ -28,6 +28,7 @@
> > #undef O_U12
> > #undef O_U16
> > #undef O_U32
> > +#undef O_U64
> >
> > #undef O_M12
> >
> > @@ -88,6 +89,11 @@
> > #undef O3_U32
> > #undef O4_U32
> >
> > +#undef O1_U64
> > +#undef O2_U64
> > +#undef O3_U64
> > +#undef O4_U64
> > +
> > #undef O1_M12
> > #undef O2_M12
> > #undef O3_M12
> > @@ -157,20 +163,21 @@
> > #define O_U12 7 /* unsigned 16 bit literal */
> > #define O_U16 8 /* unsigned 16 bit literal */
> > #define O_U32 9 /* unsigned 32 bit literal */
> > +#define O_U64 10 /* unsigned 64 bit literal */
> >
> > -#define O_M12 10 /* matches bitmask of 12 */
> > +#define O_M12 11 /* matches bitmask of 12 */
> >
> > -#define O_S2 11 /* signed 2 bit literal */
> > -#define O_S3 12 /* signed 3 bit literal */
> > -#define O_S4 13 /* signed 4 bit literal */
> > -#define O_S5 14 /* signed 5 bit literal */
> > -#define O_S8 15 /* signed 8 bit literal */
> > -#define O_S12 16 /* signed 12 bit literal */
> > -#define O_S16 17 /* signed 16 bit literal */
> > -#define O_S32 18 /* signed 32 bit literal */
> > +#define O_S2 12 /* signed 2 bit literal */
> > +#define O_S3 13 /* signed 3 bit literal */
> > +#define O_S4 14 /* signed 4 bit literal */
> > +#define O_S5 15 /* signed 5 bit literal */
> > +#define O_S8 16 /* signed 8 bit literal */
> > +#define O_S12 17 /* signed 12 bit literal */
> > +#define O_S16 18 /* signed 16 bit literal */
> > +#define O_S32 19 /* signed 32 bit literal */
> >
> > -#define O_ELEM 19 /* Element selector requiring modulo arithmetic. */
> > -#define O_LIT 20 /* Operand must be a literal fitting the target type.
> > */
> > +#define O_ELEM 20 /* Element selector requiring modulo arithmetic. */
> > +#define O_LIT 21 /* Operand must be a literal fitting the target type.
> > */
> >
> > #define O_SHIFT 5
> >
> > @@ -223,6 +230,11 @@
> > #define O3_U32 (O_U32 << (2 * O_SHIFT))
> > #define O4_U32 (O_U32 << (3 * O_SHIFT))
> >
> > +#define O1_U64 O_U64
> > +#define O2_U64 (O_U64 << O_SHIFT)
> > +#define O3_U64 (O_U64 << (2 * O_SHIFT))
> > +#define O4_U64 (O_U64 << (3 * O_SHIFT))
> > +
> > #define O1_M12 O_M12
> > #define O2_M12 (O_M12 << O_SHIFT)
> > #define O3_M12 (O_M12 << (2 * O_SHIFT))
> > @@ -1989,19 +2001,19 @@ B_DEF (s390_verllvf, vrotlv4si3,
> > 0,
> > B_DEF (s390_verllvg, vrotlv2di3, 0,
> > B_VX, 0, BT_FN_UV2DI_UV2DI_UV2DI)
> >
> > OB_DEF (s390_vec_rli, s390_vec_rli_u8,
> > s390_vec_rli_s64, B_VX, BT_FN_OV4SI_OV4SI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_u8, s390_verllb, 0,
> > 0, BT_OV_UV16QI_UV16QI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_s8, s390_verllb, 0,
> > 0, BT_OV_V16QI_V16QI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_u16, s390_verllh, 0,
> > 0, BT_OV_UV8HI_UV8HI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_s16, s390_verllh, 0,
> > 0, BT_OV_V8HI_V8HI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_u32, s390_verllf, 0,
> > 0, BT_OV_UV4SI_UV4SI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_s32, s390_verllf, 0,
> > 0, BT_OV_V4SI_V4SI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_u64, s390_verllg, 0,
> > 0, BT_OV_UV2DI_UV2DI_ULONG)
> > -OB_DEF_VAR (s390_vec_rli_s64, s390_verllg, 0,
> > 0, BT_OV_V2DI_V2DI_ULONG)
> > -
> > -B_DEF (s390_verllb, rotlv16qi3, 0,
> > B_VX, 0, BT_FN_UV16QI_UV16QI_UINT)
> > -B_DEF (s390_verllh, rotlv8hi3, 0,
> > B_VX, 0, BT_FN_UV8HI_UV8HI_UINT)
> > -B_DEF (s390_verllf, rotlv4si3, 0,
> > B_VX, 0, BT_FN_UV4SI_UV4SI_UINT)
> > -B_DEF (s390_verllg, rotlv2di3, 0,
> > B_VX, 0, BT_FN_UV2DI_UV2DI_UINT)
> > +OB_DEF_VAR (s390_vec_rli_u8, s390_verllb, 0,
> > O2_U64, BT_OV_UV16QI_UV16QI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_s8, s390_verllb, 0,
> > O2_U64, BT_OV_V16QI_V16QI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_u16, s390_verllh, 0,
> > O2_U64, BT_OV_UV8HI_UV8HI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_s16, s390_verllh, 0,
> > O2_U64, BT_OV_V8HI_V8HI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_u32, s390_verllf, 0,
> > O2_U64, BT_OV_UV4SI_UV4SI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_s32, s390_verllf, 0,
> > O2_U64, BT_OV_V4SI_V4SI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_u64, s390_verllg, 0,
> > O2_U64, BT_OV_UV2DI_UV2DI_ULONG)
> > +OB_DEF_VAR (s390_vec_rli_s64, s390_verllg, 0,
> > O2_U64, BT_OV_V2DI_V2DI_ULONG)
> > +
> > +B_DEF (s390_verllb, rotlv16qi3, 0,
> > B_VX, O2_U32, BT_FN_UV16QI_UV16QI_UINT)
> > +B_DEF (s390_verllh, rotlv8hi3, 0,
> > B_VX, O2_U32, BT_FN_UV8HI_UV8HI_UINT)
> > +B_DEF (s390_verllf, rotlv4si3, 0,
> > B_VX, O2_U32, BT_FN_UV4SI_UV4SI_UINT)
> > +B_DEF (s390_verllg, rotlv2di3, 0,
> > B_VX, O2_U32, BT_FN_UV2DI_UV2DI_UINT)
> >
> > OB_DEF (s390_vec_rl_mask,
> > s390_vec_rl_mask_s8,s390_vec_rl_mask_u64,B_VX,
> > BT_FN_OV4SI_OV4SI_OV4SI_UCHAR)
> > OB_DEF_VAR (s390_vec_rl_mask_s8, s390_verimb, 0,
> > O3_U8, BT_OV_V16QI_V16QI_UV16QI_UCHAR)
> > diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
> > index 49ab4fc7360..64f56d8effa 100644
> > --- a/gcc/config/s390/s390.cc
> > +++ b/gcc/config/s390/s390.cc
> > @@ -815,8 +815,8 @@ s390_const_operand_ok (tree arg, int argnum, int
> > op_flags, tree decl)
> > {
> > if (O_UIMM_P (op_flags))
> > {
> > - unsigned HOST_WIDE_INT bitwidths[] = { 1, 2, 3, 4, 5, 8, 12, 16, 32,
> > 4 };
> > - unsigned HOST_WIDE_INT bitmasks[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > 12 };
> > + unsigned HOST_WIDE_INT bitwidths[] = { 1, 2, 3, 4, 5, 8, 12, 16, 32,
> > 64, 4 };
> > + unsigned HOST_WIDE_INT bitmasks[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > 0, 12 };
> > unsigned HOST_WIDE_INT bitwidth = bitwidths[op_flags - O_U1];
> > unsigned HOST_WIDE_INT bitmask = bitmasks[op_flags - O_U1];
> >
> > @@ -824,7 +824,7 @@ s390_const_operand_ok (tree arg, int argnum, int
> > op_flags, tree decl)
> > gcc_assert(ARRAY_SIZE(bitmasks) == (O_M12 - O_U1 + 1));
> >
> > if (!tree_fits_uhwi_p (arg)
> > - || tree_to_uhwi (arg) > (HOST_WIDE_INT_1U << bitwidth) - 1
> > + || tree_to_uhwi (arg) > ((HOST_WIDE_INT_1U << (bitwidth - 1) << 1) -
> > 1)
> > || (bitmask && tree_to_uhwi (arg) & ~bitmask))
> > {
> > if (bitmask)
>
