[Bug middle-end/52419] New: Wrong expansion of misaligned vector store

[Bug middle-end/52419] New: Wrong expansion of misaligned vector store

[jamborm at gcc dot gnu.org]

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

             Bug #: 52419
           Summary: Wrong expansion of misaligned vector store
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jamborm@gcc.gnu.org
                CC: rguenth@gcc.gnu.org
              Host: x86_64linux-gnu
            Target: x86_64linux-gnu


Created attachment 26773
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26773
Testcase

When I compile the attached testcase, even at -O0, the resulting
binary aborts because the vector value is {5,3} instead of expected
{3,5}.  When I remove the aligned and packed attributes, the testcase
works as expected.

[Bug middle-end/52419] Wrong expansion of misaligned vector store

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #1 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-28 18:09:48 UTC ---
Jakub pointed out on IRC that the problem was elsewhere.

*** This bug has been marked as a duplicate of bug 52407 ***

[Bug target/52419] Wrong expansion of misaligned vector store

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2012-02-29
         Resolution|DUPLICATE                   |
     Ever Confirmed|0                           |1

--- Comment #2 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-29 13:19:56 UTC ---
Well, it turns out that even with Richi's patch the exact same
testcase fails at -O1 and higher.  Thus, reopening.

[Bug target/52419] Wrong expansion of misaligned vector store

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-29 13:28:00 UTC ---
Confirmed.

foo:
.LFB0:
        .cfi_startproc
        pxor    %xmm1, %xmm1
        movhps  .LC0(%rip), %xmm1
        movdqu  %xmm1, (%rdi)
        ret

it looks like 

(insn 6 5 7 (set (reg:DI 61)
        (const_int 5 [0x5])) t.c:23 -1
     (nil))

(insn 7 6 8 (set (reg:DI 62)
        (vec_select:DI (reg:V2DI 60)
            (parallel [
                    (const_int 0 [0])
                ]))) t.c:23 -1
     (nil))

(insn 8 7 9 (set (reg:V2DI 60)
        (vec_concat:V2DI (reg:DI 62)
            (reg:DI 61))) t.c:23 -1
     (nil))

(insn 9 8 0 (set (mem:V16QI (reg/v/f:DI 59 [ p ]) [0 *p_1(D)+0 S16 A8])
        (unspec:V16QI [
                (subreg:V16QI (reg:V2DI 60) 0)
            ] UNSPEC_MOVU)) t.c:23 -1
     (nil))

which of course does not work as ref:V2DI 60 is unsed uninitialized in insn 7.

[Bug target/52419] Wrong expansion of misaligned vector store

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-29 13:46:01 UTC ---
We are not prepared to handle bitsize != GET_MODE_BITSIZE in expand_assignment
for the movmisalign case.  The following fixes it

Index: gcc/expr.c
===================================================================
--- gcc/expr.c  (revision 184656)
+++ gcc/expr.c  (working copy)
@@ -4687,7 +4687,13 @@ expand_assignment (tree to, tree from, b
              != CODE_FOR_nothing))
        {
          misalignp = true;
-         to_rtx = gen_reg_rtx (mode);
+         /* If we only partly store to the misaligned memory region
+            perform a read-modify-write cycle.  Otherwise use a scratch
+            register for the 'read' part.  */
+         if (bitsize != GET_MODE_BITSIZE (mode))
+           to_rtx = expand_normal (tem);
+         else
+           to_rtx = gen_reg_rtx (mode);
        }
       else
        {

of course we then generate

foo:
.LFB0:
        .cfi_startproc
        movdqu  (%rdi), %xmm0
        movhps  .LC0(%rip), %xmm0
        movdqu  %xmm0, (%rdi)
        ret

which is rather sub-optimal.  Not sure who would be supposed to fix that.
OTOH we could as well simply avoid going the movmisalign paths when
the word is not accessed in full.  But that's harder because we'd somehow
have to circumvent the expand_normal case to fall into the movmisalign
trap.

[Bug target/52419] Wrong expansion of misaligned vector store

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-29 13:52:05 UTC ---
Failed to match this instruction:
(set (reg:DI 62)
    (vec_select:DI (subreg:V2DI (unspec:V16QI [
                    (mem:V16QI (reg/v/f:DI 59 [ p ]) [0 *p_1(D)+0 S16 A8])
                ] UNSPEC_MOVU) 0)
        (parallel [
                (const_int 0 [0])
            ])))

could at least be recognized.

[Bug target/52419] Wrong expansion of misaligned vector store

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-29 14:19:44 UTC ---
Created attachment 26785
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26785
alternative

This patch avoids expand_expr on the MEM_REF's base twice, by moving the mem
var computation earlier.

[Bug middle-end/52419] Wrong expansion of misaligned vector store

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
          Component|target                      |middle-end
   Target Milestone|---                         |4.7.0

[Bug middle-end/52419] Wrong expansion of misaligned vector store

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26785|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-29 14:53:58 UTC ---
Created attachment 26786
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26786
gcc47-pr52419.patch

Updated patch.

[Bug middle-end/52419] Wrong expansion of misaligned vector store

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-29 17:46:08 UTC ---
Author: jakub
Date: Wed Feb 29 17:45:55 2012
New Revision: 184666

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184666
Log:
    PR middle-end/52419
    * expr.c (expand_assignment): If doing misaligned store that doesn't
    cover all mode bits, perform a RMW cycle.

    * gcc.dg/torture/pr52419.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr52419.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/52419] Wrong expansion of misaligned vector store

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-29 17:52:41 UTC ---
Fixed.