[Bug regression/55451] New: FAIL: gcc.dg/fixed-point/unary.c

[Bug regression/55451] New: FAIL: gcc.dg/fixed-point/unary.c

[gretay at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

             Bug #: 55451
           Summary: FAIL: gcc.dg/fixed-point/unary.c
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: regression
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: gretay@gcc.gnu.org
                CC: jakub@gcc.gnu.org
            Target: arm-none-eabi


FAIL: gcc.dg/fixed-point/unary.c execution test

/work/nov-builds/strcmp/arm-none-eabi/gcc2/gcc/xgcc
-B/work/nov-builds/strcmp/arm-none-eabi/gcc2/gcc/
/work/local-checkouts/gcc-fsf/gcc/testsuite/gcc.dg/fixed-point/unary.c gcc_tg.o
 -fno-diagnostics-show-caret   -std=gnu99 -DSTACK_SIZE=16384      
-Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm   -o
./unary.exe    (timeout = 60)
PASS: gcc.dg/fixed-point/unary.c (test for excess errors)

*** EXIT code 4242
FAIL: gcc.dg/fixed-point/unary.c execution test

Execution fails on line 197 gcc.dg/fixed-point/unary.c, when executing the test
FRACT_SAT_NEG(_Sat long _Fract, lr);

The problem is actually in one of the files gcc automatically generates.

Caused by r193028. Author cced on this report. Reduced test attached.

./cc1 -fpreprocessed /work/bisect/_ssnegSQ.i -quiet -dumpbase _ssnegSQ.i
-mcpu=cortex-a15 -mfloat-abi=softfp -mfpu=neon-vfpv4 -auxbase-strip _ssnegSQ.o
-Wextra -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes
-Wmissing-prototypes -Wold-style-definition  -fbuilding-libgcc
-fno-stack-protector -fno-inline -fvisibility=hidden -o ssneg.s -O1 -da -dp
-fdump-tree-all

diff good/_ssnegSQ.i.081t.reassoc1 bad/_ssnegSQ.i.081t.reassoc1 
23c23
<   if (_2 < 0)
---
>   if (1 == 1)
29c29
<   if (z_3 < 0)
---
>   if (0 != 0)

[Bug regression/55451] FAIL: gcc.dg/fixed-point/unary.c

[gretay at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #1 from gretay at gcc dot gnu.org 2012-11-23 17:41:44 UTC ---
Created attachment 28766
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28766
reduced preprocessed input that causes the failure

[Bug regression/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|arm-none-eabi               |arm-none-eabi,
                   |                            |arm-linux-gnueabi
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |4.7.2
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2012-11-28
                 CC|                            |ramana at gcc dot gnu.org
     Ever Confirmed|0                           |1
            Summary|FAIL:                       |[4.8 regression] FAIL:
                   |gcc.dg/fixed-point/unary.c  |gcc.dg/fixed-point/unary.c
      Known to fail|                            |4.8.0

[Bug regression/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-30 09:56:02 UTC ---
I can't reproduce it with a cross-compiler, for me it optimizes into
  <bb 2>:
  _2 = VIEW_CONVERT_EXPR<SItype>(a_5(D));
  if (_2 == -2147483648)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 3>:
  z_3 = -_2;

  <bb 4>:
  # z_6 = PHI <2147483647(2), z_3(3)>
  _4 = VIEW_CONVERT_EXPR<SQtype>(z_6);
  return _4;
but perhaps it is related to HWI size.  That said, the bug is clearly on the
fixed-point.c side:
  SQtype c;
  SItype x, y, z;
  memcpy (&y, &a, 4);
  x = 0;
  z = x - y;
  if (((x ^ y) >> (31 + 0)) & 1)
    {
      if (((z ^ x) >> (31 + 0)) & 1)
        {
          z = 1;
          z = z << (31 + 0);
          if (x >= 0)
            z--;
        }
    }
  memcpy (&c, &z, 4);
  return c;
If this routine is called with bits 0x80000000, then z = x - y; triggers
undefined signed overflow, and afterwards testing (very weirdo very fancy way)
for the most negative value (which is the only one that triggers undefined
signed overflow) is already way too late.  Note there is another undefined
signed overflow later on, z-- where z was previously the most negative signed
integer.

So, completely untested:
--- libgcc/fixed-bit.c    2011-11-04 07:49:37.000000000 +0100
+++ libgcc/fixed-bit.c    2012-11-30 10:52:45.559780132 +0100
@@ -569,16 +569,11 @@ FIXED_SSNEG (FIXED_C_TYPE a)
   INT_C_TYPE x, y, z;
   memcpy (&y, &a, FIXED_SIZE);
   x = 0;
-  z = x - y;
+  z = x - (UINT_C_TYPE) y;
   if (((x ^ y) >> I_F_BITS) & 1)
     {
       if (((z ^ x) >> I_F_BITS) & 1)
-        {
-          z = 1;
-          z = z << I_F_BITS;
-          if (x >= 0)
-            z--;
-        }
+    z = (((UINT_C_TYPE) 1) << I_F_BITS) - 1;
     }
 #if HAVE_PADDING_BITS
   z = z << PADDING_BITS;

could fix those two undefined behaviors.  That said, the existence of the x
variable which is set to 0, then xored with various things and tested if it is
>= 0, seems very nonsensical obfuscation to me.  Why not drop x altogether, and
just use z = - (UINT_C_TYPE) y; and y >> I_F_BITS resp. z >> I_F_BITS?

[Bug regression/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[gretay at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #3 from gretay at gcc dot gnu.org 2012-11-30 17:34:44 UTC ---
I am entirely following the explanation of the failure, but the patch fixes the
testsuite regression and the assembly generated for libgcc/_ssnegSQ.o seems
correct: 

00000000 <__gnu_ssnegsq2>:
   0:    e3500000     cmp    r0, #0
   4:    e2600000     rsb    r0, r0, #0
   8:    a12fff1e     bxge    lr
   c:    e3500000     cmp    r0, #0
  10:    b3e00102     mvnlt    r0, #-2147483648    ; 0x80000000
  14:    e12fff1e     bx    lr


Thanks,
Greta

[Bug regression/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug regression/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-11 12:14:31 UTC ---
Okay, I can submit the patch to gcc-patches, but can't do testing of it easily.

[Bug libgcc/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[gretay at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #5 from gretay at gcc dot gnu.org 2012-12-11 12:28:55 UTC ---
Sorry, Jakub, I meant to say "I am *not* entirely following the explanation..." 

I can run a regression test with this patch for arm-none-eabi on qemu and/or
bootstrap on arm. Would that be enough for testing this change? 

Thanks,
Greta

[Bug libgcc/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-11 12:45:59 UTC ---
I think so.  Let me post to gcc-patches.

[Bug libgcc/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-12 09:39:01 UTC ---
Author: jakub
Date: Wed Dec 12 09:38:56 2012
New Revision: 194439

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194439
Log:
    PR libgcc/55451
    * fixed-bit.c (FIXED_SSADD, FIXED_SSSUB, FIXED_SSNEG): Avoid
    undefined signed overflows.

Modified:
    trunk/libgcc/ChangeLog
    trunk/libgcc/fixed-bit.c

[Bug libgcc/55451] [4.8 regression] FAIL: gcc.dg/fixed-point/unary.c

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55451

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-12 09:45:18 UTC ---
Hopefully fixed.