[Bug rtl-optimization/97968] New: Unnecessary mov instruction with comparison and cmov

[Bug rtl-optimization/97968] New: Unnecessary mov instruction with comparison and cmov

[denis.campredon at gmail dot com]

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

            Bug ID: 97968
           Summary: Unnecessary mov instruction with comparison and cmov
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: denis.campredon at gmail dot com
  Target Milestone: ---

The same problem applies with all comparison operators but '==' for 'int' and
'long'  on x86-64.

(Returning a negative value instead of 0 makes the compiler generate a jump
instead of a cmov. I don't know if it's worth a bug)

-------------------
int f(int n, int j)
{
    return n > j ? n : 0;
}
-------------------


with O2 produces


-------------------
f(int, int):
        mov     eax, edi
        cmp     edi, esi
        mov     edx, 0
        cmovle  eax, edx
        ret
-------------------

Ideally it should produce something like that. (the first mov can be deleted
with some little changes later)

-----------------------
f(int, int):
        mov     eax, 0
        cmp     edi, esi
        cmovg   eax, edi
        ret
-----------------------

[Bug middle-end/97968] [9/10/11/12 Regression] Unnecessary mov instruction with comparison and cmov

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.1.0, 8.1.0
      Known to work|                            |7.1.0
     Ever confirmed|0                           |1
          Component|rtl-optimization            |middle-end
           Keywords|                            |ra
   Last reconfirmed|                            |2021-09-01
            Summary|Unnecessary mov instruction |[9/10/11/12 Regression]
                   |with comparison and cmov    |Unnecessary mov instruction
                   |                            |with comparison and cmov
             Status|UNCONFIRMED                 |NEW
           Severity|normal                      |enhancement

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So looking into this, I think this is a register allocation issue. We produce:
(insn 23 6 2 2 (set (reg:SI 87)
        (reg:SI 5 di [ n ])) "/app/example.cpp":3:1 75 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 5 di [ n ])
        (nil)))
(insn 2 23 24 2 (set (reg/v:SI 83 [ n ])
        (reg:SI 87)) "/app/example.cpp":3:1 75 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 87)
        (nil)))
(insn 24 2 3 2 (set (reg:SI 88)
        (reg:SI 4 si [ j ])) "/app/example.cpp":3:1 75 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 4 si [ j ])
        (nil)))
(note 3 24 4 2 NOTE_INSN_DELETED)
(note 4 3 20 2 NOTE_INSN_FUNCTION_BEG)
(insn 20 4 25 2 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg/v:SI 83 [ n ])
            (reg:SI 88))) "/app/example.cpp":4:18 11 {*cmpsi_1}
     (expr_list:REG_DEAD (reg:SI 88)
        (nil)))
(insn 25 20 22 2 (set (reg:SI 86)
        (const_int 0 [0])) "/app/example.cpp":4:18 75 {*movsi_internal}
     (expr_list:REG_EQUIV (const_int 0 [0])
        (nil)))
(insn 22 25 17 2 (set (reg/v:SI 83 [ n ])
        (if_then_else:SI (gt (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (reg/v:SI 83 [ n ])
            (reg:SI 86))) "/app/example.cpp":4:18 1140 {*movsicc_noc}
     (expr_list:REG_DEAD (reg:SI 86)
        (expr_list:REG_DEAD (reg:CCGC 17 flags)
            (nil))))
(insn 17 22 18 2 (set (reg/i:SI 0 ax)
        (reg/v:SI 83 [ n ])) "/app/example.cpp":5:1 75 {*movsi_internal}
     (expr_list:REG_DEAD (reg/v:SI 83 [ n ])
        (nil)))

But we tie the input to the movsicc_noc to the output early on with reg 83.
that means we select 83 to be ax which means emitting a move early on instead
of selecting 86 as ax and having the second 83 be ax.


Now it looks in GCC 7 we got this before RA:
(insn 26 27 17 2 (set (reg:SI 87 [ <retval> ])
        (if_then_else:SI (gt (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (reg/v:SI 88 [ n ])
            (reg:SI 91))) "/app/example.cpp":4 968 {*movsicc_noc}
     (expr_list:REG_DEAD (reg:SI 91)
        (expr_list:REG_DEAD (reg/v:SI 88 [ n ])
            (expr_list:REG_DEAD (reg:CCGC 17 flags)
                (nil)))))
(insn 17 26 18 2 (set (reg/i:SI 0 ax)
        (reg:SI 87 [ <retval> ])) "/app/example.cpp":5 82 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 87 [ <retval> ])
        (nil)))

The change looks like is the way we expand the code from gimple to RTL. In post
GCC 8, we tie right away the return value to the register which holds n and
move  0 into n.

[Bug middle-end/97968] [9/10/11/12 Regression] Unnecessary mov instruction with comparison and cmov

[pinskia at gcc dot gnu.org]

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

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

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

[Bug middle-end/97968] [9/10/11/12 Regression] Unnecessary mov instruction with comparison and cmov

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gabravier at gmail dot com

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

[Bug middle-end/97968] [9/10/11/12 Regression] Unnecessary mov instruction with comparison and cmov

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |normal
           Priority|P3                          |P2

[Bug middle-end/97968] [9/10/11/12 Regression] Unnecessary mov instruction with comparison and cmov

[denis.campredon at gmail dot com]

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

--- Comment #3 from denis.campredon at gmail dot com ---
This seems to be fixed for gcc12, unlike the code from pr-98303

[Bug middle-end/97968] [10/11/12/13 Regression] Unnecessary mov instruction with comparison and cmov

[rguenth at gcc dot gnu.org]

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

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 middle-end/97968] [10/11/12/13 Regression] Unnecessary mov instruction with comparison and cmov

[jakub at gcc dot gnu.org]

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

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 middle-end/97968] [11/12/13/14 Regression] Unnecessary mov instruction with comparison and cmov

[rguenth at gcc dot gnu.org]

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

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

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

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

[Bug middle-end/97968] [11/12/13/14 Regression] Unnecessary mov instruction with comparison and cmov

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |needs-bisection
      Known to work|                            |13.1.0

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
In GCC 13+ we produce:
f:
        xorl    %eax, %eax
        cmpl    %esi, %edi
        cmovg   %edi, %eax
        ret

[Bug middle-end/97968] [11 Regression] Unnecessary mov instruction with comparison and cmov

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13/14 Regression]    |[11 Regression] Unnecessary
                   |Unnecessary mov instruction |mov instruction with
                   |with comparison and cmov    |comparison and cmov
                 CC|                            |law at gcc dot gnu.org

--- Comment #8 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Per c#3 and c#7.