Bug 30704 - [4.2/4.3 Regression] Incorrect constant generation for long long
Summary: [4.2/4.3 Regression] Incorrect constant generation for long long
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.2.0
: P1 blocker
Target Milestone: 4.2.0
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2007-02-05 00:56 UTC by John David Anglin
Modified: 2007-04-03 12:33 UTC (History)
6 users (show)

See Also:
Host:
Target: FLOAT_WORDS_BIG_ENDIAN targets
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-04-02 14:58:41


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John David Anglin 2007-02-05 00:56:36 UTC
The following problem was seen in a testcase under discussion in PR 30634:

# cat yy.c
#include <stdio.h>
double f(void)
{
  long long t = 0x000fffffffffffff;
  double t1;
  __builtin_memcpy(&t1, &t, sizeof(long long));
  return t1;
}
int main()
{
  union
    {
      long long ll;
      double d;
    } u;

  u.d = f ();
  printf ("ll = 0x%llx\n", u.ll);
  return 0;
}

# ../../xgcc -B../../ -O2 -o yy yy.c
# ./yy
ll = 0xffffffff000fffff
# ../../xgcc -B../../ -O0 -o yy yy.c
# ./yy
ll = 0xfffffffffffff
# ../../xgcc -B../../ -v
Reading specs from ../../specs
Target: hppa64-hp-hpux11.00
Configured with: ../gcc/configure --with-gnu-as --with-as=/opt/gnu64/bin/as
--with-ld=/usr/ccs/bin/ld --enable-shared --with-local-prefix=/opt/gnu64
--prefix=/opt/gnu64/gcc/gcc-4.2.0 --host=hppa64-hp-hpux11.00 --disable-nls
--with-gmp=/opt/gnu64/gcc/gcc-4.2.0
Thread model: posix
gcc version 4.2.0 20070123 (prerelease)

The constant 0x000fffffffffffff appears in the assembler output as a
pair of 32-bit .word values.  The values are reversed when the testcase
is compiled at -O2.  This doesn't appear to be a target issue since the
target only defines directives for the output of constants.
Comment 1 Richard Biener 2007-02-05 10:16:43 UTC
This works ok on a little-endian target.  How does the optimized tree-dump look like?  (I suppose we use VIEW_CONVERT_EXPR for the memcpy and maybe even fold
the constant therein)
Comment 2 Andrew Pinski 2007-02-05 19:43:22 UTC
Confirmed on powerpc64-linux-gnu also.
.LC0:
        .tc FD_ffffffff_fffff[TC],0xffffffff000fffff
Comment 3 David Edelsohn 2007-03-12 20:19:17 UTC
The problem appears to be occurring in real.c conversions from CONST_DOUBLE to TARGET_DOUBLE.
Comment 4 David Edelsohn 2007-03-12 22:20:51 UTC
I do not believe this is an endian issue.  It is a coincidence that the output value looks like the original constant.  GCC converts the __builtin_memcpy() into a VIEW_CONVERT_EXPR<double>.  The constant is equivalent to a NaN and GCC uses the value CONST_DOUBLE NaN, not the original bits.  real.c converts the CONST_DOUBLE NaN into a ieee_double NaN, which happens to look like the value printed.  On little endian targets, the words are swapped, which just happens to look like the original constant.

If the VIEW_CONVERT_EXPR<double> is valid, manipulating it like a NaN seems valid.
Comment 5 Andrew Pinski 2007-03-12 22:25:30 UTC
Index: ../../gcc/fold-const.c
===================================================================
--- ../../gcc/fold-const.c      (revision 122864)
+++ ../../gcc/fold-const.c      (working copy)
@@ -7357,6 +7357,7 @@ native_interpret_real (tree type, unsign
      up to 192 bits.  */
   REAL_VALUE_TYPE r;
   long tmp[6];
+  long tmp1[6];

   total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
   if (total_bytes > len || total_bytes > 24)
@@ -7386,6 +7387,11 @@ native_interpret_real (tree type, unsign
     }

   real_from_target (&r, tmp, mode);
+  real_to_target (tmp1, &r, mode);
+  /* If the encoded real is going to decode differently, then reject the constant
+     conversion. */
+  if (memcpy (tmp, tmp1,  total_bytes) != 0)
+    return NULL;
   return build_real (type, r);
 }
Comment 6 Andrew Pinski 2007-03-12 22:38:08 UTC
From native_encode_int, we get:
(gdb) p/x *(unsigned int[2]*)ptr
$14 = {0xffff0f00, 0xffffffff}

Which is obviously wrong, it should have encoded it as:
{0x000fffff, 0xffffffff}

So we get the wrong answer to begin with and it is not an issue with native_interpret_real, at least right at this moment.
Comment 7 Andrew Pinski 2007-03-12 22:44:22 UTC
(In reply to comment #6)
> From native_encode_int, we get:
> (gdb) p/x *(unsigned int[2]*)ptr
> $14 = {0xffff0f00, 0xffffffff}

No that is right, I was looking at while doing a cross so (unsigned int[2]*) meant we have byte endian swap.
Comment 8 Andrew Pinski 2007-03-12 23:08:33 UTC
the issue is with native_interpret_real really, tmp is getting the wrong words order.
Comment 9 Andrew Pinski 2007-03-12 23:18:52 UTC
Here is my new patch:
Index: ../../gcc/fold-const.c
===================================================================
--- ../../gcc/fold-const.c      (revision 122864)
+++ ../../gcc/fold-const.c      (working copy)
@@ -7357,6 +7357,7 @@ native_interpret_real (tree type, unsign
      up to 192 bits.  */
   REAL_VALUE_TYPE r;
   long tmp[6];
+  long tmp1[6];

   total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
   if (total_bytes > len || total_bytes > 24)
@@ -7367,6 +7368,7 @@ native_interpret_real (tree type, unsign
   for (byte = 0; byte < total_bytes; byte++)
     {
       int bitpos = byte * BITS_PER_UNIT;
+      int bytepos;
       if (total_bytes > UNITS_PER_WORD)
        {
          word = byte / UNITS_PER_WORD;
@@ -7381,11 +7383,18 @@ native_interpret_real (tree type, unsign
       else
        offset = BYTES_BIG_ENDIAN ? (total_bytes - 1) - byte : byte;
       value = ptr[offset];
+      bytepos = FLOAT_WORDS_BIG_ENDIAN ? total_bytes / 4 - bitpos / 32 - 1 : bitpos / 32;

-      tmp[bitpos / 32] |= (unsigned long)value << (bitpos & 31);
+      tmp[bytepos] |= (unsigned long)value << (bitpos & 31);
+      printf("%ld %ld [%d,%d]\n", tmp[0], tmp[1], bytepos, bitpos);
     }

   real_from_target (&r, tmp, mode);
+  real_to_target (tmp1, &r, mode);
+  /* If the encoded real is going to decode differently, then reject the constant
+     conversion. */
+  if (memcmp (tmp, tmp1, total_bytes) != 0)
+    return NULL;
   return build_real (type, r);
 }



I am not 100 % sure this is correct but we get the correct constant now for this one.
Comment 10 Andrew Pinski 2007-03-12 23:21:15 UTC
I proved to my self at least with a cross from x86 to ppc64, I get the correct output:
change t:
  long long t = 0x1234567890123456ULL;

and we get:
        .tc FD_12345678_90123456[TC],0x1234567890123456
Comment 11 Andrew Pinski 2007-03-13 00:10:18 UTC
Mine, my patch is correct, just need to test it now.
Comment 12 Andrew Pinski 2007-03-13 22:16:11 UTC
Another testcase but it is not fixed by my patch as I did not fix the other side:
int main(void)
{
  double b = 234.0;
  long long c;
  double d = b;
  __builtin_memcpy(&c, &b, sizeof(double));
  long long e = c;
  if (__builtin_memcmp (&e, &d, sizeof(double)) != 0)
    __builtin_abort ();
  return 0;
}
Comment 13 Andrew Pinski 2007-03-14 06:08:59 UTC
I can't get my last testcase in comment #12 working.
Roger this was caused by your folding of VIEW_CONVERT_EXPR patch.
Comment 14 Jakub Jelinek 2007-04-03 10:05:50 UTC
Subject: Bug 30704

Author: jakub
Date: Tue Apr  3 10:05:38 2007
New Revision: 123455

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=123455
Log:
	PR middle-end/30704
	* fold-const.c (native_encode_real): Encode real.c provided longs
	as a series of 32-bit native integers.
	(native_interpret_real): Interpret buffer as a series of 32-bit
	native integers.

	* gcc.c-torture/execute/ieee/pr30704.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/ieee/pr30704.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

Comment 15 Jakub Jelinek 2007-04-03 10:23:19 UTC
Subject: Bug 30704

Author: jakub
Date: Tue Apr  3 10:23:03 2007
New Revision: 123460

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=123460
Log:
	PR middle-end/30704
	* fold-const.c (native_encode_real): Encode real.c provided longs
	as a series of 32-bit native integers.
	(native_interpret_real): Interpret buffer as a series of 32-bit
	native integers.

	* gcc.c-torture/execute/ieee/pr30704.c: New test.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/ieee/pr30704.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/fold-const.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog

Comment 16 Jakub Jelinek 2007-04-03 12:33:36 UTC
Fixed.