[Bug target/104039] New: AArch64 Redundant instruction moving general to vector register

[Bug target/104039] New: AArch64 Redundant instruction moving general to vector register

[nate at thatsmathematics dot com]

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

            Bug ID: 104039
           Summary: AArch64 Redundant instruction moving general to vector
                    register
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nate at thatsmathematics dot com
  Target Milestone: ---

Compiling the following code on AArch64 with -O2 or -O3:

typedef unsigned long u64x2 __attribute__((vector_size(16)));

u64x2 combine(unsigned long a, unsigned long b) {
        u64x2 v = {a,b};
    return v;
}

yields the following assembly:

combine:
        fmov    d0, x0
        ins     v0.d[1], x1
        ins     v0.d[1], x1
        ret

where the second ins is entirely redundant with the first and serves no
apparent purpose.  (Unless it is something extremely clever...)

This seems to be a regression from 8.x to 9.x; Godbolt's 8.5 looks correct with
just one ins, but 9.3 has the two.

Originally noticed by Peter Cordes on StackOverflow:
https://stackoverflow.com/questions/70717360/how-to-load-vector-registers-from-integer-registers-in-arm64-m1/70718572#comment125016906_70717360

[Bug target/104039] AArch64 Redundant instruction moving general to vector register

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2022-01-15
     Ever confirmed|0                           |1
             Target|                            |aarch64
           Keywords|                            |ra
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
The problem is the register allocator.
It goes from:
(insn 18 10 19 2 (set (reg:DI 99)
        (reg:DI 0 x0 [ a ])) "/app/example.cpp":4:49 53 {*movdi_aarch64}
     (expr_list:REG_DEAD (reg:DI 0 x0 [ a ])
        (nil)))
(insn 19 18 8 2 (set (reg:DI 100)
        (reg:DI 1 x1 [ b ])) "/app/example.cpp":4:49 53 {*movdi_aarch64}
     (expr_list:REG_DEAD (reg:DI 1 x1 [ b ])
        (nil)))
(insn 8 19 15 2 (set (reg:V2DI 95 [ a ])
        (subreg:V2DI (reg:DI 99) 0)) "/app/example.cpp":5:16 1206
{*aarch64_simd_movv2di}
     (expr_list:REG_DEAD (reg:DI 99)
        (nil)))
To:
(insn 8 10 15 2 (set (reg:V2DI 32 v0 [orig:95 a ] [95])
        (reg:V2DI 0 x0 [99])) "/app/example.cpp":5:16 1206
{*aarch64_simd_movv2di}
     (nil))

And then that gets split into:
(insn 22 10 23 2 (set (reg:V2DI 32 v0 [orig:95 a ] [95])
        (vec_concat:V2DI (reg:DI 0 x0 [99])
            (const_int 0 [0]))) "/app/example.cpp":5:16 1919
{move_lo_quad_internal_v2di}
     (nil))
(insn 23 22 15 2 (set (reg:V2DI 32 v0 [orig:95 a ] [95])
        (vec_concat:V2DI (vec_select:DI (reg:V2DI 32 v0 [orig:95 a ] [95])
                (parallel:V2DI [
                        (const_int 0 [0])
                    ]))
            (reg:DI 1 x1 [+8 ]))) "/app/example.cpp":5:16 1932
{aarch64_simd_move_hi_quad_v2di}
     (nil))

Maybe we should have done the expansion to use an fmov in the first place via a
special pattern which then gets expanded correctly might be better, I don't
know.

[Bug target/104039] AArch64 Redundant instruction moving general to vector register

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |9.1.0
           Severity|enhancement                 |normal
   Target Milestone|---                         |9.5
      Known to work|                            |8.5.0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So back in GCC 8.5 we used dup to create the initial vector and then did an
insert. for floating point registers this works fine.
But if we do:
typedef double  f64x2 __attribute__((vector_size(16)));

f64x2 combine(double a, double b) {
    asm("// %0 %1":"+r"(a));
        f64x2 v = {a,b};
    return v;
}

We get two ins which we should not get.
While in GCC 8.5.0 we got:
        fmov    x0, d0
// x0 x0
        fmov    d0, x0
        dup     v0.2d, v0.d[0]
        ins     v0.d[1], v1.d[0]

Which is also wrong because there is no reason for a fmov/dup, just fmov is
enough.

[Bug target/104039] [9/10/11/12 Regression] AArch64 Redundant instruction moving general to vector register

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/104039] [9/10/11/12 Regression] AArch64 Redundant instruction moving general to vector register

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

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

--- Comment #3 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
This has been fixed in GCC-12 where we optimize the merge of two vec_duplicates
into a simple vec_concat. https://godbolt.org/z/dWYfnjKnq

can we drop the 12 regression?

[Bug target/104039] [10/11/12/13 Regression] AArch64 Redundant instruction moving general to vector register

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

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

[Bug target/104039] [10/11/12/13 Regression] AArch64 Redundant instruction moving general to vector register

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug target/104039] [10/11 Regression] AArch64 Redundant instruction moving general to vector register

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11/12/13 Regression]    |[10/11 Regression] AArch64
                   |AArch64 Redundant           |Redundant instruction
                   |instruction moving general  |moving general to vector
                   |to vector register          |register
             Status|NEW                         |RESOLVED
      Known to work|                            |12.1.0, 13.0
   Target Milestone|10.5                        |12.0
         Resolution|---                         |FIXED

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed for GCC 12.