public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char
@ 2020-07-12 23:49 gabravier at gmail dot com
  2020-07-13 10:48 ` [Bug target/96176] " jakub at gcc dot gnu.org
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: gabravier at gmail dot com @ 2020-07-12 23:49 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 96176
           Summary: Failure to omit extraneous movzx in atomic compare
                    exchange with unsigned char
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gabravier at gmail dot com
  Target Milestone: ---

void f(unsigned char *addr, unsigned char old_val, unsigned char new_val)
{
    __atomic_compare_exchange_n(addr, &old_val, new_val, 0, 0, 0);
}

On x86 with -O3, LLVM generates this :

f(unsigned char*, unsigned char, unsigned char): # @f(unsigned char*, unsigned
char, unsigned char)
  mov eax, esi
  lock cmpxchg byte ptr [rdi], dl
  ret

GCC generates this :

f(unsigned char*, unsigned char, unsigned char):
  mov eax, esi
  movzx edx, dl
  lock cmpxchg BYTE PTR [rdi], dl
  ret

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug target/96176] Failure to omit extraneous movzx in atomic compare exchange with unsigned char
  2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
@ 2020-07-13 10:48 ` jakub at gcc dot gnu.org
  2020-07-13 10:48 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-07-13 10:48 UTC (permalink / raw)
  To: gcc-bugs

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

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> ---
The zero extension is initially there because the builtin has the arguments
promoted.  Normally combine would fix this up:
(insn 10 7 11 2 (set (reg:SI 88 [ new_val ])
        (zero_extend:SI (subreg:QI (reg:SI 93) 0))) "pr96176.c":3:5 143
{*zero_extendqisi2}
     (expr_list:REG_DEAD (reg:SI 93)
        (nil)))
(insn 11 10 0 2 (parallel [
            (set (reg:QI 89)
                (unspec_volatile:QI [
                        (mem/v:QI (reg/v/f:DI 83 [ addr ]) [-1  S1 A8])
                        (reg/v:QI 84 [ old_val ])
                        (subreg:QI (reg:SI 88 [ new_val ]) 0)
                        (const_int 0 [0])
                    ] UNSPECV_CMPXCHG))
            (set (mem/v:QI (reg/v/f:DI 83 [ addr ]) [-1  S1 A8])
                (unspec_volatile:QI [
                        (const_int 0 [0])
                    ] UNSPECV_CMPXCHG))
            (set (reg:CCZ 17 flags)
                (unspec_volatile:CCZ [
                        (const_int 0 [0])
                    ] UNSPECV_CMPXCHG))
        ]) "pr96176.c":3:5 6114 {atomic_compare_and_swapqi_1}
     (expr_list:REG_DEAD (reg:SI 88 [ new_val ])
        (expr_list:REG_DEAD (reg/v:QI 84 [ old_val ])
            (expr_list:REG_DEAD (reg/v/f:DI 83 [ addr ])
                (expr_list:REG_UNUSED (reg:QI 89)
                    (expr_list:REG_UNUSED (reg:CCZ 17 flags)
                        (nil)))))))

but it fails:
Trying 10 -> 11:
   10: r88:SI=zero_extend(r93:SI#0)
      REG_DEAD r93:SI
   11: {r89:QI=unspec/v[[r83:DI],r84:QI,r88:SI#0,0] 90;[r83:DI]=unspec/v[0]
90;flags:CCZ=unspec/v[0] 90;}
      REG_DEAD r88:SI
      REG_DEAD r84:QI
      REG_DEAD r83:DI
      REG_UNUSED r89:QI
      REG_UNUSED flags:CCZ
Failed to match this instruction:
(parallel [
        (set (reg:QI 89)
            (unspec_volatile:QI [
                    (mem/v:QI (reg/v/f:DI 83 [ addr ]) [-1  S1 A8])
                    (reg/v:QI 84 [ old_val ])
                    (subreg:QI (reg:SI 93) 0)
                    (const_int 0 [0])
                ] UNSPECV_CMPXCHG))
        (set (mem/v:QI (reg/v/f:DI 83 [ addr ]) [-1  S1 A8])
            (unspec_volatile:QI [
                    (const_int 0 [0])
                ] UNSPECV_CMPXCHG))
        (set (reg:CCZ 17 flags)
            (unspec_volatile:CCZ [
                    (const_int 0 [0])
                ] UNSPECV_CMPXCHG))
    ])
due to the MEM_VOLATILE_P and volatile_ok not being true during combine.
But we can do something about it during expansion.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug target/96176] Failure to omit extraneous movzx in atomic compare exchange with unsigned char
  2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
  2020-07-13 10:48 ` [Bug target/96176] " jakub at gcc dot gnu.org
@ 2020-07-13 10:48 ` jakub at gcc dot gnu.org
  2020-07-15  9:29 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-07-13 10:48 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-07-13
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48868
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48868&action=edit
gcc11-pr96176.patch

Untested fix.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug target/96176] Failure to omit extraneous movzx in atomic compare exchange with unsigned char
  2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
  2020-07-13 10:48 ` [Bug target/96176] " jakub at gcc dot gnu.org
  2020-07-13 10:48 ` jakub at gcc dot gnu.org
@ 2020-07-15  9:29 ` cvs-commit at gcc dot gnu.org
  2020-07-15 10:00 ` jakub at gcc dot gnu.org
  2024-01-20 17:16 ` pinskia at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-07-15  9:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 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:410675cb63466d8de9ad590521f0766b012d2475

commit r11-2103-g410675cb63466d8de9ad590521f0766b012d2475
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jul 15 11:26:22 2020 +0200

    builtins: Avoid useless char/short -> int promotions before atomics
[PR96176]

    As mentioned in the PR, we generate a useless movzbl insn before lock
cmpxchg.
    The problem is that the builtin for the char/short cases has the arguments
    promoted to int and combine gives up, because the instructions have
    MEM_VOLATILE_P arguments and recog in that case doesn't recognize anything
    when volatile_ok is false, and nothing afterwards optimizes the
    (reg:SI a) = (zero_extend:SI (reg:QI a))
    ... (subreg:QI (reg:SI a) 0) ...

    The following patch fixes it at expansion time, we already have a function
    that is meant to undo the promotion, so this just adds the very common case
    to that.

    2020-07-15  Jakub Jelinek  <jakub@redhat.com>

            PR target/96176
            * builtins.c: Include gimple-ssa.h, tree-ssa-live.h and
            tree-outof-ssa.h.
            (expand_expr_force_mode): If exp is a SSA_NAME with different mode
            from MODE and get_gimple_for_ssa_name is a cast from MODE, use the
            cast's rhs.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug target/96176] Failure to omit extraneous movzx in atomic compare exchange with unsigned char
  2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
                   ` (2 preceding siblings ...)
  2020-07-15  9:29 ` cvs-commit at gcc dot gnu.org
@ 2020-07-15 10:00 ` jakub at gcc dot gnu.org
  2024-01-20 17:16 ` pinskia at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-07-15 10:00 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug target/96176] Failure to omit extraneous movzx in atomic compare exchange with unsigned char
  2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
                   ` (3 preceding siblings ...)
  2020-07-15 10:00 ` jakub at gcc dot gnu.org
@ 2024-01-20 17:16 ` pinskia at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-20 17:16 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-20 17:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-12 23:49 [Bug target/96176] New: Failure to omit extraneous movzx in atomic compare exchange with unsigned char gabravier at gmail dot com
2020-07-13 10:48 ` [Bug target/96176] " jakub at gcc dot gnu.org
2020-07-13 10:48 ` jakub at gcc dot gnu.org
2020-07-15  9:29 ` cvs-commit at gcc dot gnu.org
2020-07-15 10:00 ` jakub at gcc dot gnu.org
2024-01-20 17:16 ` pinskia at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).