[Bug rtl-optimization/65401] New: [5 Regression] make_field_assignment broken for big-endian

[Bug rtl-optimization/65401] New: [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

            Bug ID: 65401
           Summary: [5 Regression] make_field_assignment broken for
                    big-endian
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org

struct S { unsigned short s[64]; };

__attribute__((noinline, noclone)) void
foo (struct S *x)
{
  unsigned int i;
  unsigned char *s;

  s = (unsigned char *) x->s;
  for (i = 0; i < 64; i++)
    x->s[i] = s[i * 2] | (s[i * 2 + 1] << 8);
}  

int
main ()
{
  unsigned int i;
  struct S s;
  for (i = 0; i < 64; i++)
    s.s[i] = i + ((64 - i) << 8);
  foo (&s);
  for (i = 0; i < 64; i++)
    if (s.s[i] != (64 - i) + (i << 8))
      __builtin_abort ();
  return 0;
}

is miscompiled on s390x-linux with -m64 -march=z9-109 -mtune=z10 -O2 -fPIC
since r220249, but most likely has been latent before that.
make_field_assignment during combine misoptimizes:
(set (mem:HI (plus:DI (reg/v/f:DI 126 [ x ])
            (reg:DI 125 [ ivtmp.11 ])) [2 MEM[base: x_3(D), index: ivtmp.11_41,
offset: 0B]+0 S2 A16])
    (ior:HI (zero_extend:HI (mem:QI (plus:DI (reg/v/f:DI 126 [ x ])
                    (reg:DI 125 [ ivtmp.11 ])) [0 MEM[base: s_4, index:
ivtmp.11_41, offset: 0B]+0 S1 A8]))
        (subreg:HI (reg:SI 128 [ D.1536 ]) 2)))
into:
(set (mem:QI (plus:DI (reg/v/f:DI 126 [ x ])
            (reg:DI 125 [ ivtmp.11 ])) [2 MEM[base: x_3(D), index: ivtmp.11_41,
offset: 0B]+0 S1 A16])
    (subreg:QI (lshiftrt:SI (reg:SI 128 [ D.1536 ])
            (const_int 8 [0x8])) 3))
which is just wrong for big-endian.

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-03-12
   Target Milestone|---                         |5.0
     Ever confirmed|0                           |1

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, not latent, that make_field_assignment stuff has been added in r220249.
So, either we need to disable it altogether for anything but little endian, or
adjust the addresses when adjusting MEMs mode too.

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[law at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

--- Comment #2 from Jeffrey A. Law <law at redhat dot com> ---
I was in this code recently, so mine.

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
In the
9345      else if (GET_CODE (rhs) == AND
9346           && paradoxical_subreg_p (XEXP (rhs, 0))
9347           && GET_CODE (SUBREG_REG (XEXP (rhs, 0))) == MEM
9348           && CONST_INT_P (XEXP (rhs, 1))
9349           && rtx_equal_for_field_assignment_p (gen_rtx_MEM (GET_MODE
(dest),
9350                                     XEXP (SUBREG_REG (XEXP (rhs, 0)), 0)),
9351                            dest))
hunks (two similar ones) sometimes GET_MODE (SUBREG_REG (XEXP (rhs, 0))) is
== GET_MODE (dest), then it is certainly safe for big endian and you shouldn't
need to even create a new MEM.
For the other case, I'd say when increasing the mode size you really need to
adjust the address of the mem for endianity, dunno if it is possible to have
also narrower dest than the SUBREG_REG MEM, in that case punt?
Oh, and please use MEM_P instead of GET_CODE () == MEM.

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Sat Mar 14 08:54:08 2015
New Revision: 221433

URL: https://gcc.gnu.org/viewcvs?rev=221433&root=gcc&view=rev
Log:
    PR rtl-optimization/65401
    * combine.c (rtx_equal_for_field_assignment_p): Add widen_x
    argument.  If true, adjust_address_nv of x with big-endian
    correction for the mode widening to GET_MODE (y).
    (make_field_assignment): Don't do MEM mode widening here.
    Use MEM_P instead of GET_CODE == MEM.

    * gcc.c-torture/execute/pr65401.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr65401.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/combine.c
    trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/65401] [5 Regression] make_field_assignment broken for big-endian

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65401

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
           Assignee|law at redhat dot com              |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.