[Bug tree-optimization/96167] New: fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[Bug tree-optimization/96167] New: fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[nok.raven at gmail dot com]

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

            Bug ID: 96167
           Summary: fails to detect ROL pattern in simple case, but
                    succeeds when operand goes through memcpy
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nok.raven at gmail dot com
  Target Milestone: ---

inline void swap(int &x, int &y)
{
  int tmp = x;
  x = y;
  y = tmp;
}

void foo(int (&x)[2])
{
  swap(x[0], x[1]);
}

void bar(int (&x)[2])
{
  int y[2];
  __builtin_memcpy(&y, &x, sizeof x);
  swap(y[0], y[1]);
  __builtin_memcpy(&x, &y, sizeof x);
}


foo:
  movl (%rdi), %eax
  movl 4(%rdi), %edx
  movl %eax, 4(%rdi)
  movl %edx, (%rdi)
  ret

bar:
  rolq $32, (%rdi)
  ret

https://godbolt.org/z/Tcz7YG

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |tree-optimization
             Target|                            |x86_64-*-* i?86-*-*
           Keywords|                            |missed-optimization
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
store-merging to the rescue?

  tmp_3 = MEM[(int &)x_1(D)];
  _4 = MEM[(int &)x_1(D) + 4];
  MEM[(int &)x_1(D)] = _4;
  MEM[(int &)x_1(D) + 4] = tmp_3;

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2020-07-15
     Ever confirmed|0                           |1

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
store-merging certainly has the infrastructure and does it job until:

  /* A complete byte swap should make the symbolic number to start with
     the largest digit in the highest order byte.  Unchanged symbolic
     number indicates a read with same endianness as target architecture.  */
  if (n.n != cmpnop && n.n != cmpxchg)
    return false;

where it of course realizes that this is not a proper 64-bit bswap.

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah.  But at least if the target has corresponding mode rotate optab, there is
no reason not to support any rotates by constant multiples of byte size (for
bit rotates we don't have infrastructure for that).

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Like:
unsigned long long
foo (unsigned long long x)
{
  union U { unsigned long long x; char y[8]; } u, v;
  u.x = x;
  v.y[0] = u.y[7];
  v.y[1] = u.y[0];
  v.y[2] = u.y[1];
  v.y[3] = u.y[2];
  v.y[4] = u.y[3];
  v.y[5] = u.y[4];
  v.y[6] = u.y[5];
  v.y[7] = u.y[6];
  return v.x;
}

unsigned long long
bar (unsigned long long x)
{
  union U { unsigned long long x; char y[8]; } u;
  u.x = x;
  char t = u.y[7];
  u.y[7] = u.y[6];
  u.y[6] = u.y[5];
  u.y[5] = u.y[4];
  u.y[4] = u.y[3];
  u.y[3] = u.y[2];
  u.y[2] = u.y[1];
  u.y[1] = u.y[0];
  u.y[0] = t;
  return u.x;
}

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |12.1.0
      Known to fail|                            |10.5.0

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
For x86_64, this was fixed in GCC 11+ at -O3 and for GCC 12+ for -O2 by the
vectorizer.

But I think we could do better at the code generation at the gimple level.
We have:
  vect__4.7_8 = MEM <vector(2) int> [(int &)x_1(D)];
  vect_tmp_3.8_9 = VEC_PERM_EXPR <vect__4.7_8, vect__4.7_8, { 1, 0 }>;
  MEM <vector(2) int> [(int &)x_1(D)] = vect_tmp_3.8_9;

Which could just be:
tmp = MEM<long>[(int&)x_1(D)];
tmp = tmp ror 32;
MEM<long>[(int&)x_1(D)] = tmp;

[Bug tree-optimization/96167] fails to detect ROL pattern in simple case, but succeeds when operand goes through memcpy

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 93721 has been marked as a duplicate of this bug. ***