[Bug target/97004] New: [nvptx] __int128 initializer incorrect

[Bug target/97004] New: [nvptx] __int128 initializer incorrect

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97004

            Bug ID: 97004
           Summary: [nvptx] __int128 initializer incorrect
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider this code from c-c++-common/spec-barrier-1.c:
...
#ifdef __SIZEOF_INT128__
__int128 g = 9;
#endif
...

We seem to be generating:
...
// BEGIN GLOBAL VAR DEF: g
.visible .global .align 8 .u64 g[2] =
{9,9 };
...

[Bug target/97004] [nvptx] __int128 initializer incorrect

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97004

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Tentative patch:
...
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 0376ad6ce9f..26868590322 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -2054,7 +2054,11 @@ nvptx_assemble_value (unsigned HOST_WIDE_INT val,
unsigned size)

   for (unsigned part = 0; size; size -= part)
     {
-      val >>= part * BITS_PER_UNIT;
+      if (part == sizeof (val))
+       /* Avoid undefined behaviour.  */
+       val = 0;
+      else
+       val >>= (part * BITS_PER_UNIT);
       part = init_frag.size - init_frag.offset;
       part = MIN (part, size);

...

And we have:
...
// BEGIN GLOBAL VAR DEF: g
        .visible .global .align 8 .u64 g[2] = { 9, 0 };
...

[Bug target/97004] [nvptx] __int128 initializer incorrect

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97004

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #1)
> Tentative patch:
> ...
> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
> index 0376ad6ce9f..26868590322 100644
> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -2054,7 +2054,11 @@ nvptx_assemble_value (unsigned HOST_WIDE_INT val,
> unsigned size)
>  
>    for (unsigned part = 0; size; size -= part)
>      {
> -      val >>= part * BITS_PER_UNIT;
> +      if (part == sizeof (val))

That assumes that CHAR_BIT == BITS_PER_UNIT, which is mostly the case, but
shouldn't be relied on.
Perhaps just use
val >>= (part * BITS_PER_UNIT / 2);
val >>= (part * BITS_PER_UNIT / 2);
or if (part * BITS_PER_UNIT == sizeof (val) * CHAR_BIT)
Though, as val is unsigned HOST_WIDE_INT, we already have a macro for that,
so if (part * BITS_PER_UNIT == HOST_BITS_PER_WIDE_INT) ?

[Bug target/97004] [nvptx] __int128 initializer incorrect

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97004

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@gcc.gnu.org>:

https://gcc.gnu.org/g:af47a2035a4882e6d4506e3d00b5a42414e3ee2b

commit r11-3127-gaf47a2035a4882e6d4506e3d00b5a42414e3ee2b
Author: Tom de Vries <tdevries@suse.de>
Date:   Thu Sep 10 15:54:14 2020 +0200

    [nvptx] Fix printing of 128-bit constant

    Currently, for this code from c-c++-common/spec-barrier-1.c:
    ...
    __int128 g = 9;
    ...
    we generate:
    ...
    // BEGIN GLOBAL VAR DEF: g
    .visible .global .align 8 .u64 g[2] = { 9, 9 };
    ...
    and consequently the test-case fails in execution.

    The problem is caused by a shift in nvptx_assemble_value:
    ...
          val >>= part * BITS_PER_UNIT;
    ...
    where the shift amount is equal to the number of bits in val, which is
    undefined behaviour.

    Fix this by detecting the situation and setting val to 0.

    Tested on nvptx.

    gcc/ChangeLog:

            PR target/97004
            * config/nvptx/nvptx.c (nvptx_assemble_value): Handle shift by
            number of bits in shift operand.

[Bug target/97004] [nvptx] __int128 initializer incorrect

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97004

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
   Target Milestone|---                         |11.0

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> (In reply to Tom de Vries from comment #1)
> > Tentative patch:
> > ...
> > diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
> > index 0376ad6ce9f..26868590322 100644
> > --- a/gcc/config/nvptx/nvptx.c
> > +++ b/gcc/config/nvptx/nvptx.c
> > @@ -2054,7 +2054,11 @@ nvptx_assemble_value (unsigned HOST_WIDE_INT val,
> > unsigned size)
> >  
> >    for (unsigned part = 0; size; size -= part)
> >      {
> > -      val >>= part * BITS_PER_UNIT;
> > +      if (part == sizeof (val))
> 
> That assumes that CHAR_BIT == BITS_PER_UNIT, which is mostly the case, but
> shouldn't be relied on.
> Perhaps just use
> val >>= (part * BITS_PER_UNIT / 2);
> val >>= (part * BITS_PER_UNIT / 2);
> or if (part * BITS_PER_UNIT == sizeof (val) * CHAR_BIT)
> Though, as val is unsigned HOST_WIDE_INT, we already have a macro for that,
> so if (part * BITS_PER_UNIT == HOST_BITS_PER_WIDE_INT) ?

I went with the last suggestion, thanks for the review.

Patch committed to trunk, no test-case required.

Marking resolved-fixed.