diff mbox series

[committed,nvptx] Fix printing of 128-bit constant (negative case)

Message ID 2fbf4c72-5039-62f7-bd7e-0fcc2180a7e5@suse.de
State New
Headers show
Series [committed,nvptx] Fix printing of 128-bit constant (negative case) | expand

Commit Message

Tom de Vries Sept. 11, 2020, 5:32 a.m. UTC 
[ was: Re: [committed][nvptx] Fix printing of 128-bit constant ]

On 9/10/20 10:26 PM, Jakub Jelinek wrote:
> On Thu, Sep 10, 2020 at 09:31:33PM +0200, Tom de Vries wrote:
>> 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.
> 
> Actually, looking more at it, is nvptx_assemble_value called from
> nvptx_assemble_integer with the CONST_INT and size of 16?
> Then
>   val &= ((unsigned  HOST_WIDE_INT)2 << (size * BITS_PER_UNIT - 1)) - 1;
> will invoke UB too (perhaps it can just do:
>   if (size * BITS_PER_UNIT < HOST_BITS_PER_WIDE_INT)
>     val &= HOST_WIDE_INT_1U << (size * BITS_PER_UNIT);
> ?), and also, TImode CONST_INTs are considered signed, i.e.
> you want value of -1 rather than 0 if the MSB (of the original value before
> any shifting) is set.
> And, will it only ever trigger == equality of the part, or could it e.g.
> emit it even in more fragments and have the shift count >
> HOST_BITS_PER_WIDE_INT?
> So check what you get for
> __int128 h = -9;

Negative __int128 indeed had a problem as well.

Fixed in patch below, committed.

Thanks,
- Tom
diff mbox series

Patch

[nvptx] Fix printing of 128-bit constant (negative case)

For this code:
...
__int128 min_one = -1;
...
we currently generate:
...
.visible .global .align 8 .u64 min_one[2] = { -1, 0 };
...

Fix this in nvptx_assemble_value, such that we have instead:
...
.visible .global .align 8 .u64 min_one[2] = { -1, -1 };
...

gcc/ChangeLog:

	* config/nvptx/nvptx.c (nvptx_assemble_value): Handle negative
	__int128.

gcc/testsuite/ChangeLog:

	* gcc.target/nvptx/int128.c: New test.

---
 gcc/config/nvptx/nvptx.c                |  5 ++++-
 gcc/testsuite/gcc.target/nvptx/int128.c | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index ffcbe591a8f..4fca0ed76d9 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -2050,13 +2050,16 @@  output_init_frag (rtx sym)
 static void
 nvptx_assemble_value (unsigned HOST_WIDE_INT val, unsigned size)
 {
+  bool negative_p
+    = val & (HOST_WIDE_INT_1U << (HOST_BITS_PER_WIDE_INT - 1));
+
   val &= ((unsigned  HOST_WIDE_INT)2 << (size * BITS_PER_UNIT - 1)) - 1;
 
   for (unsigned part = 0; size; size -= part)
     {
       if (part * BITS_PER_UNIT == HOST_BITS_PER_WIDE_INT)
 	/* Avoid undefined behaviour.  */
-	val = 0;
+	val = negative_p ? -1 : 0;
       else
 	val >>= (part * BITS_PER_UNIT);
       part = init_frag.size - init_frag.offset;
diff --git a/gcc/testsuite/gcc.target/nvptx/int128.c b/gcc/testsuite/gcc.target/nvptx/int128.c
new file mode 100644
index 00000000000..069dbbfe12b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/nvptx/int128.c
@@ -0,0 +1,15 @@ 
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-pedantic" } */
+
+__int128 one = 1;
+__int128 min_one = -1;
+__int128 zero = 0;
+
+int
+main (void)
+{
+  if (zero - one != min_one)
+    __builtin_abort ();
+
+  return 0;
+}