[Bug c/37809] New: Incorrect code with MMX right shift __builtin_ia32_psradi

[Bug c/37809] New: Incorrect code with MMX right shift __builtin_ia32_psradi

[suckfish at ihug dot co dot nz]

Bug on both trunk and 4.3.2 [for the latter, change psradi to psrad as the
builtin function name changed].

Compiling the following code with  -O2 -flax-vector-conversions -mmmx -msse
-msse2  generates incorrect code:


typedef int v2si __attribute__ ((vector_size (8)));
v2si foo (v2si x)
{
    x &= (v2si) 0xffffffffll;
    x = __builtin_ia32_psradi (x, 1);
    x &= (v2si) 0x80000000ll;
    return x;
}


The compile incorrectly assumes that bit 31 of the shift result always ends up
as zero:

foo:
        pxor    %xmm0, %xmm0
        ret


Presumeably, some optimiseration is incorrectly treating the v2si right shift
as a 64-bit right shift.

[It appears to me that both nonzero_bits1 and num_sign_bit_copies1 can
incorrectly optimise vector operation as if they were scalar ops, which is what
inspired this example, however, attempting to fix those did not fix this bug,
which makes me think the problem is elsewhere.]


-- 
           Summary: Incorrect code with MMX right shift
                    __builtin_ia32_psradi
           Product: gcc
           Version: 4.3.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: suckfish at ihug dot co dot nz
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


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

[Bug c/37809] Incorrect code with MMX right shift __builtin_ia32_psradi

[suckfish at ihug dot co dot nz]

------- Comment #1 from suckfish at ihug dot co dot nz  2008-10-12 02:47 -------
Created an attachment (id=16483)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16483&action=view)
Test case as a complete program.


-- 


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

[Bug target/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-10-12 10:35 -------
Confirmed.  Works on i686.  Worked with 4.1.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |target
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
      Known to fail|                            |4.2.4 4.3.2 4.4.0
      Known to work|                            |4.1.2
   Last reconfirmed|0000-00-00 00:00:00         |2008-10-12 10:35:59
               date|                            |
            Summary|Incorrect code with MMX     |[4.2/4.3/4.4 Regression]
                   |right shift                 |Incorrect code with MMX
                   |__builtin_ia32_psradi       |right shift
                   |                            |__builtin_ia32_psradi
   Target Milestone|---                         |4.2.5


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

[Bug target/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[ubizjak at gmail dot com]

------- Comment #3 from ubizjak at gmail dot com  2008-10-12 19:00 -------
Mine.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ubizjak at gmail dot com
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-10-12 10:35:59         |2008-10-12 19:00:22
               date|                            |


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

[Bug target/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[suckfish at ihug dot co dot nz]

------- Comment #4 from suckfish at ihug dot co dot nz  2008-10-14 07:17 -------
Digging a bit, in combine.c, the ASHIFTRT case of force_to_mode() contains two
calls to simplify_shift_const().  Disabling those for vector modes fixes the
test case here (patch below).

But I suspect this is just the tip of the iceberg; there are probably many
other arithmetic simplifications here that will get incorrectly applied to
vector types, especially when sizeof(vector type) <= sizeof (HOST_WIDE_INT).

Do we need to audit the whole compiler for such things, or is there a sensible
place we can insert a don't-optimise-vector-types test with disabling too many
useful optimisations?

Thinking that the test-suite probably contains many more tests for 128 bit
vector types, would it be possible/worth-while to build a compiler & run the
test-suite with HOST_WIDE_INT being 128 bits?


diff --git a/gcc/combine.c b/gcc/combine.c
index 5821301..ad24d94 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7635,7 +7635,8 @@
              nonzero >>= INTVAL (XEXP (x, 1));
            }

-         if ((mask & ~nonzero) == 0)
+         if ((mask & ~nonzero) == 0
+              && !VECTOR_MODE_P (mode) && !VECTOR_MODE_P (GET_MODE (x)))
            {
              x = simplify_shift_const (NULL_RTX, LSHIFTRT, GET_MODE (x),
                                        XEXP (x, 0), INTVAL (XEXP (x, 1)));
@@ -7643,7 +7644,8 @@
                return force_to_mode (x, mode, mask, next_select);
            }

-         else if ((i = exact_log2 (mask)) >= 0)
+         else if ((i = exact_log2 (mask)) >= 0
+                   && !VECTOR_MODE_P (mode) && !VECTOR_MODE_P (GET_MODE (x)))
            {
              x = simplify_shift_const
                  (NULL_RTX, LSHIFTRT, GET_MODE (x), XEXP (x, 0),


-- 


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

[Bug target/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[ubizjak at gmail dot com]

------- Comment #5 from ubizjak at gmail dot com  2008-10-17 06:35 -------
(In reply to comment #4)
> Digging a bit, in combine.c, the ASHIFTRT case of force_to_mode() contains two
> calls to simplify_shift_const().  Disabling those for vector modes fixes the
> test case here (patch below).

Can you please post the patch with correct ChangeLog to gcc-patches mailing
list?
Also, the discussion about patch should start in gcc-patches ML.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|ubizjak at gmail dot com    |unassigned at gcc dot gnu
                   |                            |dot org
             Status|ASSIGNED                    |NEW


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

[Bug middle-end/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[suckfish at ihug dot co dot nz]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 934 bytes --]



------- Comment #6 from suckfish at ihug dot co dot nz  2008-10-17 22:18 -------
Pleasantly, my fears that this was just the tip of the iceburg seems to be
wrong.  Writing some test cases, found another bug involving a shift, and also
the ICE below [fixed by some combination of change above and 38707 fix.]

Will fix the remaining case & post patches to gcc-patches list.

#include <mmintrin.h>
typedef int v2si __attribute__ ((vector_size (8)));
v2si shiftU (v2si y)
{
    return _m_psrldi (_m_psrldi (y, 0), 0);
}

$ gcc -O shift2.c
shift2.c: In function ‘shiftU’:
shift2.c:6: internal compiler error: in trunc_int_for_mode, at explow.c:55
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
Preprocessed source stored into /tmp/cckqI2oC.out file, please attach this to
your bugreport.


-- 


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

[Bug middle-end/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


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

[Bug middle-end/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[suckfish at ihug dot co dot nz]

------- Comment #7 from suckfish at ihug dot co dot nz  2008-11-10 07:44 -------
Could someone commit the patch? 
[http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00147.html]   I don't have SVN
commit access.


-- 


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

[Bug middle-end/37809] [4.2/4.3/4.4 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[uros at gcc dot gnu dot org]

------- Comment #8 from uros at gcc dot gnu dot org  2008-11-10 09:09 -------
Subject: Bug 37809

Author: uros
Date: Mon Nov 10 09:08:15 2008
New Revision: 141732

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141732
Log:
        PR middle-end/37807
        PR middle-end/37809
        * combine.c (force_to_mode): Do not process vector types.

        * rtlanal.c (nonzero_bits1): Do not process vector types.
        (num_sign_bit_copies1): Likewise.

testsuite/ChangeLog

        PR middle-end/37807
        PR middle-end/37809
        * gcc/testsuite/gcc.target/i386/mmx-8.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/mmx-8.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/combine.c
    trunk/gcc/rtlanal.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug middle-end/37809] [4.2/4.3 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[uros at gcc dot gnu dot org]

------- Comment #9 from uros at gcc dot gnu dot org  2008-11-10 10:45 -------
Subject: Bug 37809

Author: uros
Date: Mon Nov 10 10:43:35 2008
New Revision: 141734

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141734
Log:
        PR middle-end/37807
        PR middle-end/37809
        * combine.c (force_to_mode): Do not process vector types.

        * rtlanal.c (nonzero_bits1): Do not process vector types.
        (num_sign_bit_copies1): Likewise.

testsuite/ChangeLog:

        PR middle-end/37807
        PR middle-end/37809
        * gcc/testsuite/gcc.target/i386/mmx-8.c: New test.


Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.target/i386/mmx-8.c
      - copied unchanged from r141732,
trunk/gcc/testsuite/gcc.target/i386/mmx-8.c
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/combine.c
    branches/gcc-4_3-branch/gcc/rtlanal.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug middle-end/37809] [4.2/4.3 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[uros at gcc dot gnu dot org]

------- Comment #10 from uros at gcc dot gnu dot org  2008-11-10 12:22 -------
Subject: Bug 37809

Author: uros
Date: Mon Nov 10 12:20:55 2008
New Revision: 141737

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141737
Log:
        Backport from mainline:
        2008-11-10  Ralph Loader  <suckfish@ihug.co.nz>

        PR middle-end/37807
        PR middle-end/37809
        * combine.c (force_to_mode): Do not process vector types.

        * rtlanal.c (nonzero_bits1): Do not process vector types.
        (num_sign_bit_copies1): Likewise.

testsuite/ChangeLog:

        Backport from mainline:
        2008-11-10  Ralph Loader  <suckfish@ihug.co.nz>

        PR middle-end/37807
        PR middle-end/37809
        * gcc/testsuite/gcc.target/i386/mmx-8.c: New test.


Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.target/i386/mmx-8.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/combine.c
    branches/gcc-4_2-branch/gcc/rtlanal.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug middle-end/37809] [4.2/4.3 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[ubizjak at gmail dot com]

------- Comment #11 from ubizjak at gmail dot com  2008-11-10 12:24 -------
Fixed everywhere.


-- 

ubizjak at gmail dot com changed:

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


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

[Bug middle-end/37809] [4.2/4.3 Regression] Incorrect code with MMX right shift __builtin_ia32_psradi

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.2.5                       |4.3.3


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