public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Alexander Monakov <amonakov@ispras.ru>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] doc: clarify semantics of vector bitwise shifts
Date: Thu, 25 May 2023 08:50:29 +0200	[thread overview]
Message-ID: <CAFiYyc39GdcPegXWj4_FJ_XCKtK73ytmGgum_HYnU=V2q3imAw@mail.gmail.com> (raw)
In-Reply-To: <4b477e8c-f8e6-5587-23ed-540f0c28ea05@ispras.ru>

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.  Note we have

/* Shift operations for shift and rotate.
   Shift means logical shift if done on an
   unsigned type, arithmetic shift if done on a signed type.
   The second operand is the number of bits to
   shift by; it need not be the same type as the first operand and result.
   Note that the result is undefined if the second operand is larger
   than or equal to the first operand's type size.

   The first operand of a shift can have either an integer or a
   (non-integer) fixed-point type.  We follow the ISO/IEC TR 18037:2004
   semantics for the latter.

   Rotates are defined for integer types only.  */
DEFTREECODE (LSHIFT_EXPR, "lshift_expr", tcc_binary, 2)

in tree.def which implies short << 24 is undefined behavior (similar
wording in generic.texi).  The rtl docs say nothing about behavior
but I think the semantics should carry over.  That works for x86
even for scalar instructions working on GPRs (masking is applied
but fixed to 5 or 6 bits even for QImode or HImode shifts).

Note that when we make these shifts well-defined there's
also arithmetic on signed types smaller than int (which again
doesn't exist in C) where overflow invokes undefined behavior
in the middle-end.  Unless we want to change that as well
this is somewhat inconsistent then.

There's also the issue that C 'int' is defined by INT_TYPE_SIZE
and thus target dependent which makes what is undefined and
what not target dependent.

Richard.

> #include <stdint.h>
>
> #define CHECK(TYPE, WIDTH, OP, COUNT, INVERT)         \
> {                                                     \
> typedef TYPE vec __attribute__((vector_size(WIDTH))); \
>                                                       \
>         static volatile vec zero;                     \
>         vec tmp = (zero-2) OP (COUNT);                \
>         vec ref = INVERT zero;                        \
>         if (__builtin_memcmp(&tmp, &ref, sizeof tmp)) \
>                 __builtin_abort();                    \
> }
>
> int main(void)
> {
>         CHECK( uint8_t, 16, <<, 8,  )
>         CHECK( uint8_t, 16, <<, 31, )
>         CHECK( uint8_t, 16, >>, 8,  )
>         CHECK( uint8_t, 16, >>, 31, )
>         CHECK(  int8_t, 16, <<, 8,  )
>         CHECK(  int8_t, 16, <<, 31, )
>         CHECK(  int8_t, 16, >>, 8,  ~)
>         CHECK(  int8_t, 16, >>, 31, ~)
>         CHECK(uint16_t, 16, <<, 16, )
>         CHECK(uint16_t, 16, <<, 31, )
>         CHECK(uint16_t, 16, >>, 16, )
>         CHECK(uint16_t, 16, >>, 31, )
>         CHECK( int16_t, 16, <<, 16, )
>         CHECK( int16_t, 16, <<, 31, )
>         CHECK( int16_t, 16, >>, 16, ~)
>         CHECK( int16_t, 16, >>, 31, ~)
>         // Per-lane-variable shifts:
>         CHECK( uint8_t, 16, <<, zero+8,  )
>         CHECK( uint8_t, 16, <<, zero+31, )
>         CHECK( uint8_t, 16, >>, zero+8,  )
>         CHECK( uint8_t, 16, >>, zero+31, )
>         CHECK(  int8_t, 16, <<, zero+8,  )
>         CHECK(  int8_t, 16, <<, zero+31, )
>         CHECK(  int8_t, 16, >>, zero+8,  ~)
>         CHECK(  int8_t, 16, >>, zero+31, ~)
>         CHECK(uint16_t, 16, <<, zero+16, )
>         CHECK(uint16_t, 16, <<, zero+31, )
>         CHECK(uint16_t, 16, >>, zero+16, )
>         CHECK(uint16_t, 16, >>, zero+31, )
>         CHECK( int16_t, 16, <<, zero+16, )
>         CHECK( int16_t, 16, <<, zero+31, )
>         CHECK( int16_t, 16, >>, zero+16, ~)
>         CHECK( int16_t, 16, >>, zero+31, ~)
>
>         // Repeat for WIDTH=32 and WIDTH=64
> }
>
> Alexander

  reply	other threads:[~2023-05-25  6:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-24 16:28 Richard Biener
2023-05-24 18:36 ` Alexander Monakov
2023-05-25  6:50   ` Richard Biener [this message]
2023-05-25 10:46     ` Richard Biener
2023-05-30 14:49     ` Alexander Monakov
2023-05-31  7:12       ` Richard Biener
2023-06-01 18:25         ` Alexander Monakov
2023-06-02  7:07           ` Matthias Kretz
2023-06-02  7:49             ` Alexander Monakov
2023-06-02  9:03               ` Matthias Kretz
2023-06-02  9:24                 ` Alexander Monakov
2023-06-02  9:34                   ` Matthias Kretz
2023-06-02  9:36                   ` Richard Biener
2023-06-02  9:39           ` Richard Biener
  -- strict thread matches above, loose matches on Subject: below --
2023-05-24 12:53 Alexander Monakov
2023-05-24 13:21 ` Richard Biener
2023-05-24 14:21   ` Alexander Monakov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFiYyc39GdcPegXWj4_FJ_XCKtK73ytmGgum_HYnU=V2q3imAw@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=amonakov@ispras.ru \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).