Re: [PATCH] doc: clarify semantics of vector bitwise shifts

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Jun 1, 2023 at 8:25 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>
>
> On Wed, 31 May 2023, Richard Biener wrote:
>
> > On Tue, May 30, 2023 at 4:49 PM Alexander Monakov <amonakov@ispras.ru> wrote:
> > >
> > >
> > > On Thu, 25 May 2023, Richard Biener wrote:
> > >
> > > > On Wed, May 24, 2023 at 8:36 PM Alexander Monakov <amonakov@ispras.ru> wrote:
> > > > >
> > > > >
> > > > > On Wed, 24 May 2023, Richard Biener via Gcc-patches wrote:
> > > > >
> > > > > > I’d have to check the ISAs what they actually do here - it of course depends
> > > > > > on RTL semantics as well but as you say those are not strictly defined here
> > > > > > either.
> > > > >
> > > > > Plus, we can add the following executable test to the testsuite:
> > > >
> > > > Yeah, that's probably a good idea.  I think your documentation change
> > > > with the added sentence about the truncation is OK.
> > >
> > > I am no longer confident in my patch, sorry.
> > >
> > > My claim about vector shift semantics in OpenCL was wrong. In fact it specifies
> > > that RHS of a vector shift is masked to the exact bitwidth of the element type.
> > >
> > > So, to collect various angles:
> > >
> > > 1. OpenCL semantics would need an 'AND' before a shift (except VSX/Altivec).
> > >
> > > 2. From user side we had a request to follow C integer promotion semantics
> > >    in https://gcc.gnu.org/PR91838 but I now doubt we can do that.
> > >
> > > 3. LLVM makes oversized vector shifts UB both for 'vector_size' and
> > >    'ext_vector_type'.
> >
> > I had the impression GCC desired to do 3. as well, matching what we do
> > for scalar shifts.
> >
> > > 4. Vector lowering does not emit promotions, and starting from gcc-12
> > >    ranger treats oversized shifts according to the documentation you
> > >    cite below, and optimizes (e.g. with '-O2 -mno-sse')
> > >
> > >         typedef short v8hi __attribute__((vector_size(16)));
> > >
> > >         void f(v8hi *p)
> > >         {
> > >             *p >>= 16;
> > >         }
> > >
> > >    to zeroing '*p'. If this looks unintended, I can file a bug.
> > >
> > > I still think we need to clarify semantics of vector shifts, but probably
> > > not in the way I proposed initially. What do you think?
> >
> > I think the intent at some point was to adhere to the OpenCL spec
> > for the GCC vector extension (because that's a written spec while
> > GCCs vector extension docs are lacking).  Originally the powerpc
> > altivec 'vector' keyword spurred most of the development IIRC
> > so it might be useful to see how they specify shifts.
>
> It doesn't look like they document the semantics of '<<' and '>>'
> operators for vector types.
>
> > So yes, we probably should clarify the semantics to match the
> > implementation (since we have two targets doing things differently
> > since forever we can only document it as UB) and also note the
> > difference from OpenCL (in case OpenCL is still relevant these
> > days we might want to offer a -fopencl-vectors to emit the required
> > AND).
>
> It doesn't have to be UB, in principle we could say that shift amount
> is taken modulo some power of two depending on the target without UB.
> But since LLVM already treats that as UB, we might as well follow.
>
> I think for addition/multiplication of signed vectors everybody
> expects them to have wrapping semantics without UB on overflow though?

Actually GCC already treats them as UB on overflow by means of
vector lowering eventually turning them into scalar operations and
quite some patterns in match.pd applying to ANY_INTEGRAL_TYPE_P.

> Revised patch below.

The revised patch is OK.

Thanks,
Richard.

> > It would be also good to amend the RTL documentation.
> >
> > It would be very nice to start an internals documentation section
> > around collecting what the middle-end considers undefined
> > or implementation defined (aka target defined) behavior in the
> > GENERIC, GIMPLE and RTL ILs and what predicates eventually
> > control that (like TYPE_OVERFLOW_UNDEFINED).  Maybe spread it over
> > {gimple,generic,rtl}.texi, though gimple.texi is only about the representation
> > and all semantics are shared and documented in generic.texi.
>
> Hm, noted. Thanks.
>
> ---8<---
>
> From e4e8d9e262f2f8dbc91a94291cf7accb74d27e7c Mon Sep 17 00:00:00 2001
> From: Alexander Monakov <amonakov@ispras.ru>
> Date: Wed, 24 May 2023 15:48:29 +0300
> Subject: [PATCH] doc: clarify semantics of vector bitwise shifts
>
> Explicitly say that attempted shift past element bit width is UB for
> vector types.  Mention that integer promotions do not happen.
>
> gcc/ChangeLog:
>
>         * doc/extend.texi (Vector Extensions): Clarify bitwise shift
>         semantics.
> ---
>  gcc/doc/extend.texi | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index e426a2eb7d..3723cfe467 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -12026,7 +12026,14 @@ elements in the operand.
>  It is possible to use shifting operators @code{<<}, @code{>>} on
>  integer-type vectors. The operation is defined as following: @code{@{a0,
>  a1, @dots{}, an@} >> @{b0, b1, @dots{}, bn@} == @{a0 >> b0, a1 >> b1,
> -@dots{}, an >> bn@}}@. Vector operands must have the same number of
> +@dots{}, an >> bn@}}@.  Unlike OpenCL, values of @code{b} are not
> +implicitly taken modulo bit width of the base type @code{B}, and the behavior
> +is undefined if any @code{bi} is greater than or equal to @code{B}.
> +
> +In contrast to scalar operations in C and C++, operands of integer vector
> +operations do not undergo integer promotions.
> +
> +Operands of binary vector operations must have the same number of
>  elements.
>
>  For convenience, it is allowed to use a binary vector operation
> --
> 2.39.2