[Bug rtl-optimization/94857] New: Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[Bug rtl-optimization/94857] New: Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[gabravier at gmail dot com]

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

            Bug ID: 94857
           Summary: Failure to optimize load+add+store into add on memory
                    when getting carry flag afterwards on x86
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gabravier at gmail dot com
  Target Milestone: ---

bool f(unsigned *p, unsigned x)
{
    unsigned u = *p;
    *p += x;
    return u > *p;
}

With -O3, LLVM outputs :

f(unsigned int*, unsigned int): # @f(unsigned int*, unsigned int)
  add dword ptr [rdi], esi
  setb al
  ret

GCC outputs :

f(unsigned int*, unsigned int):
  add esi, DWORD PTR [rdi]
  mov DWORD PTR [rdi], esi
  setc al
  ret

[Bug target/94857] Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-04-30
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Likely because we expand from

  u_4 = *p_3(D);
  _8 = .ADD_OVERFLOW (u_4, x_5(D));
  _1 = REALPART_EXPR <_8>;
  _9 = IMAGPART_EXPR <_8>;
  *p_3(D) = _1;
  _7 = _9 != 0;
  return _7;

and add-overflow expansion does not know how to use the TERed u_4.  The
pattern is the following and allows memory:

(define_insn "*add<mode>3_cc_overflow_1"
  [(set (reg:CCC FLAGS_REG)
        (compare:CCC
            (plus:SWI
                (match_operand:SWI 1 "nonimmediate_operand" "%0,0")
                (match_operand:SWI 2 "<general_operand>" "<r><i>,m"))
            (match_dup 1)))
   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
        (plus:SWI (match_dup 1) (match_dup 2)))]
  "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)"
  "add{<imodesuffix>}\t{%2, %0|%0, %2}"
  [(set_attr "type" "alu")
   (set_attr "mode" "<MODE>")])

but maybe the alternatives are mixed up here in the different insn parts
of the parallel ...

[Bug target/94857] Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The add-overflow expansion, while it could through TER see the memory load,
doesn't see the uses (and there could be multiple of them).
The combiner doesn't deal with this because the insn has multiple SETs (flags
and a pseudo).
I think this is usually handled through peephole2s and in this particular case
can be handled that way, though perhaps we could get better results if combine
or some other pre-RA pass could deal with the cases of MEM stores from insn
with multiple sets.

[Bug target/94857] Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48476
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48476&action=edit
gcc1-pr94857.patch

Untested patch that handles it through peephole2.

[Bug target/94857] Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r11-197-ga229f9b3737062c6e853879be6683f3f3e4a6661
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 8 10:03:56 2020 +0200

    ix86: Add peephole2 for *add<mode>3_cc_overflow_1 followed by matching
memory store [PR94857]

    The following peephole2 changes:
    -       addl    (%rdi), %esi
    +       xorl    %eax, %eax
    +       addl    %esi, (%rdi)
            setc    %al
    -       movl    %esi, (%rdi)
    -       movzbl  %al, %eax
            ret
    on the testcase.  *add<mode>3_cc_overflow_1, being an add{l,q} insn, is
    commutative, so if TARGET_READ_MODIFY_WRITE we can replace
    addl (%rdi), %esi; movl %esi, (%rdi)
    with
    addl %esi, (%rdi)
    if %esi is dead after those two insns.

    2020-05-08  Jakub Jelinek  <jakub@redhat.com>

            PR target/94857
            * config/i386/i386.md (peephole2 after *add<mode>3_cc_overflow_1):
New
            define_peephole2.

            * gcc.target/i386/pr94857.c: New test.

[Bug target/94857] Failure to optimize load+add+store into add on memory when getting carry flag afterwards on x86

[jakub at gcc dot gnu.org]

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

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

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

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