[Bug target/108933] New: [10/11/12/13 Regression] Missing bswap detection

[Bug target/108933] New: [10/11/12/13 Regression] Missing bswap detection

[pinskia at gcc dot gnu.org]

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

            Bug ID: 108933
           Summary: [10/11/12/13 Regression] Missing bswap detection
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
                CC: ktkachov at gcc dot gnu.org, unassigned at gcc dot gnu.org
        Depends on: 108874
            Blocks: 94094
  Target Milestone: ---
            Target: arm

This is the arm version of this bug.


+++ This bug was initially created as a clone of Bug #108874 +++


If we look at the arm testcases in gcc.target/arm/rev16.c
typedef unsigned int __u32;

__u32
__rev16_32_alt (__u32 x)
{
  return (((__u32)(x) & (__u32)0xff00ff00UL) >> 8)
         | (((__u32)(x) & (__u32)0x00ff00ffUL) << 8);
}

__u32
__rev16_32 (__u32 x)
{
  return (((__u32)(x) & (__u32)0x00ff00ffUL) << 8)
         | (((__u32)(x) & (__u32)0xff00ff00UL) >> 8);
}

we should be able to generate rev16 instructions for aarch64 (and arm) i.e.
recognise a __builtin_bswap16 essentially.
GCC fails to do so and generates:
__rev16_32_alt:
        lsr     w1, w0, 8
        lsl     w0, w0, 8
        and     w1, w1, 16711935
        and     w0, w0, -16711936
        orr     w0, w1, w0
        ret
__rev16_32:
        lsl     w1, w0, 8
        lsr     w0, w0, 8
        and     w1, w1, -16711936
        and     w0, w0, 16711935
        orr     w0, w1, w0
        ret

whereas clang manages to recognise it all into:
__rev16_32_alt:                         // @__rev16_32_alt
        rev16   w0, w0
        ret
__rev16_32:                             // @__rev16_32
        rev16   w0, w0
        ret

does the bswap pass need some tweaking perhaps?

Looks like this worked fine with GCC 5 but broke in the GCC 6 timeframe so
marking as a regression


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94094
[Bug 94094] [meta-bug] store-merging and/or bswap load/store-merging missed
optimizations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108874
[Bug 108874] [10/11/12/13 Regression] Missing bswap detection

[Bug target/108933] [10/11/12/13 Regression] Missing bswap detection

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.5

[Bug target/108933] [10/11/12/13 Regression] Missing bswap detection

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108933
Bug 108933 depends on bug 108874, which changed state.

Bug 108874 Summary: [10/11/12/13 Regression] Missing bswap detection
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108874

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

[Bug target/108933] [10/11/12/13 Regression] Missing bswap detection

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
As mentioned in the other PR, this isn't a bswap.  If the bswap pass should
optimize this, there would need to be a new optab for this and corresponding
internal function,
which bswap patch could use if the optab is defined.

[Bug target/108933] [10/11/12/13 Regression] Missing rev16 detection

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11/12/13 Regression]    |[10/11/12/13 Regression]
                   |Missing bswap detection     |Missing rev16 detection

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #1)
> As mentioned in the other PR, this isn't a bswap.  If the bswap pass should
> optimize this, there would need to be a new optab for this and corresponding
> internal function,
> which bswap patch could use if the optab is defined.

Sorry I forgot to change the summary here. Anyways yes rev16 is not bswap.

[Bug target/108933] [10/11/12/13 Regression] Missing rev16 detection

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also note the arm fix will be very similar to what was done for aarch64 just I
have no way to test the change so I didn't implement it ...

[Bug target/108933] [10/11/12/13 Regression] Missing rev16 detection

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-03-27
           Priority|P3                          |P2
     Ever confirmed|0                           |1

[Bug target/108933] [11/12/13/14 Regression] Missing rev16 detection

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.

[Bug target/108933] [11/12/13/14 Regression] Missing rev16 detection

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Earnshaw <rearnsha@gcc.gnu.org>:

https://gcc.gnu.org/g:bad991a1c5960e90c4686a9362a1258ef29e195b

commit r14-8499-gbad991a1c5960e90c4686a9362a1258ef29e195b
Author: Matthieu Longo <matthieu.longo@arm.com>
Date:   Mon Jan 29 15:54:35 2024 +0000

    arm: Add pattern for bswap + rotate -> rev16 [Bug 108933]

    The rev16 pattern was not recognised anymore as a change in the bswap
    tree pass was introducing a new GIMPLE form, not recognized by the
    assembly final transformation pass.

    Also, fix the output patterns for arm_rev16si_alt[12] to correctly
    handle the instructions being made conditional.

    More details in the PR.

    gcc/ChangeLog:

            PR target/108933
            * config/arm/arm.md (arm_rev16si2): Convert to define_insn.
            Correct generated RTL.
            (arm_rev16si2_alt1): Correctly handle conditional execution.
            (arm_rev16si2_alt2): Likewise.

    gcc/testsuite/ChangeLog:

            PR target/108933
            * gcc.target/arm/rev16.c: Moved to...
            * gcc.target/arm/rev16_1.c: ...here.
            * gcc.target/arm/rev16_2.c: New test to check that rev16 is
emitted.

[Bug target/108933] [11/12/13 Regression] Missing rev16 detection

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13/14 Regression]    |[11/12/13 Regression]
                   |Missing rev16 detection     |Missing rev16 detection
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |Matthieu.Longo at arm dot com

--- Comment #6 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Fixed on trunk so far.