* [Bug target/97004] [nvptx] __int128 initializer incorrect
2020-09-09 22:39 [Bug target/97004] New: [nvptx] __int128 initializer incorrect vries at gcc dot gnu.org
@ 2020-09-10 13:24 ` vries at gcc dot gnu.org
2020-09-10 13:34 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-10 13:24 UTC (permalink / raw)
To: gcc-bugs
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 };
...
^ permalink raw reply [flat|nested] 5+ messages in thread* [Bug target/97004] [nvptx] __int128 initializer incorrect
2020-09-09 22:39 [Bug target/97004] New: [nvptx] __int128 initializer incorrect vries at gcc dot gnu.org
2020-09-10 13:24 ` [Bug target/97004] " vries at gcc dot gnu.org
@ 2020-09-10 13:34 ` jakub at gcc dot gnu.org
2020-09-10 19:30 ` cvs-commit at gcc dot gnu.org
2020-09-10 19:33 ` vries at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-10 13:34 UTC (permalink / raw)
To: gcc-bugs
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) ?
^ permalink raw reply [flat|nested] 5+ messages in thread* [Bug target/97004] [nvptx] __int128 initializer incorrect
2020-09-09 22:39 [Bug target/97004] New: [nvptx] __int128 initializer incorrect vries at gcc dot gnu.org
2020-09-10 13:24 ` [Bug target/97004] " vries at gcc dot gnu.org
2020-09-10 13:34 ` jakub at gcc dot gnu.org
@ 2020-09-10 19:30 ` cvs-commit at gcc dot gnu.org
2020-09-10 19:33 ` vries at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-09-10 19:30 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* [Bug target/97004] [nvptx] __int128 initializer incorrect
2020-09-09 22:39 [Bug target/97004] New: [nvptx] __int128 initializer incorrect vries at gcc dot gnu.org
` (2 preceding siblings ...)
2020-09-10 19:30 ` cvs-commit at gcc dot gnu.org
@ 2020-09-10 19:33 ` vries at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-10 19:33 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 5+ messages in thread