[Bug target/53020] New: __atomic_fetch_or doesn't generate `1 insn` variant

[Bug target/53020] New: __atomic_fetch_or doesn't generate `1 insn` variant

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

             Bug #: 53020
           Summary: __atomic_fetch_or doesn't generate `1 insn` variant
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: kirill.yukhin@intel.com


Hello,
while working on Intel's TSX extensions, I've found strange (to me) thing.

We have in config/i386/sync.md:
(define_insn "atomic_<code><mode>"
  [(set (match_operand:SWI 0 "memory_operand" "+m")
        (unspec_volatile:SWI
          [(any_logic:SWI (match_dup 0)
                          (match_operand:SWI 1 "nonmemory_operand" "<r><i>"))
           (match_operand:SI 2 "const_int_operand")]            ;; model
          UNSPECV_LOCK))
...

any_logic covers (unconditionally) covers AND IOR and XOR ops.

However, generated insn-opinit.c lacks IOR variant initalization:
...
set_direct_optab_handler (atomic_and_optab, QImode, CODE_FOR_atomic_andqi);
set_direct_optab_handler (atomic_xor_optab, QImode, CODE_FOR_atomic_xorqi);
...

So, having such simple test:
void
foo (int *p, int v)
{
  __atomic_fetch_or (p, 1, __ATOMIC_ACQUIRE | __ATOMIC_HLE_ACQUIRE);
}

`lock orl      %edx, (%eax)` wont' be generated, since there is no
corresponding occurence in IOR optab.
Here is the code, that fails to find it:
optabs.c:maybe_emit_op
...
      if (use_memmodel)
        {
          icode = direct_optab_handler (optab->mem_no_result, mode);
...

The most strange thing to me is that it works fine with XOR and AND ops.

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

--- Comment #1 from Yukhin Kirill <kirill.yukhin at intel dot com> 2012-04-17 16:23:26 UTC ---
Instead, of single `locked` instruction, it generates:.L2:
        movl    %eax, %ecx
        orl     $1, %ecx
        lock cmpxchgl   %ecx, (%edx)
Similar variant for AND operation:
        lock andl      %edx, (%eax)

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-04-17
         AssignedTo|unassigned at gcc dot       |ubizjak at gmail dot com
                   |gnu.org                     |
   Target Milestone|---                         |4.5.4
     Ever Confirmed|0                           |1

--- Comment #2 from Uros Bizjak <ubizjak at gmail dot com> 2012-04-17 16:54:06 UTC ---
Uh...

Index: config/i386/sync.md
===================================================================
--- config/i386/sync.md (revision 186501)
+++ config/i386/sync.md (working copy)
@@ -576,7 +576,7 @@
   return "lock{%;} sub{<imodesuffix>}\t{%1, %0|%0, %1}";
 })

-(define_insn "atomic_<code><mode>"
+(define_insn "atomic_<logic><mode>"
   [(set (match_operand:SWI 0 "memory_operand" "+m")
        (unspec_volatile:SWI
          [(any_logic:SWI (match_dup 0)

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

--- Comment #3 from Yukhin Kirill <kirill.yukhin at intel dot com> 2012-04-17 17:00:34 UTC ---
(In reply to comment #2)
> Uh...
> 
> Index: config/i386/sync.md
> ===================================================================
> --- config/i386/sync.md (revision 186501)
> +++ config/i386/sync.md (working copy)
> @@ -576,7 +576,7 @@
>    return "lock{%;} sub{<imodesuffix>}\t{%1, %0|%0, %1}";
>  })
> 
> -(define_insn "atomic_<code><mode>"
> +(define_insn "atomic_<logic><mode>"
>    [(set (match_operand:SWI 0 "memory_operand" "+m")
>         (unspec_volatile:SWI
>           [(any_logic:SWI (match_dup 0)

Oh, I see. Thanks!

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[uros at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

--- Comment #5 from uros at gcc dot gnu.org 2012-04-17 17:39:12 UTC ---
Author: uros
Date: Tue Apr 17 17:39:06 2012
New Revision: 186543

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186543
Log:
    PR target/53020
    * config/i386/sync.md (atomic_<code><mode>): Rename to
    atomic_<logic><mode>.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/i386/sync.md

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[uros at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

--- Comment #4 from uros at gcc dot gnu.org 2012-04-17 17:35:30 UTC ---
Author: uros
Date: Tue Apr 17 17:35:23 2012
New Revision: 186542

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186542
Log:
    PR target/53020
    * config/i386/sync.md (atomic_<code><mode>): Rename to
    atomic_<logic><mode>.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/sync.md

[Bug target/53020] __atomic_fetch_or doesn't generate `1 insn` variant

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53020

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2012-04/msg01032.htm
                   |                            |l
         Resolution|                            |FIXED
   Target Milestone|4.5.4                       |4.7.1

--- Comment #6 from Uros Bizjak <ubizjak at gmail dot com> 2012-04-17 17:55:49 UTC ---
Fixed.

Please note that many other targets got the named pattern wrong...