[Bug c/99998] New: Unnecessary jump instruction

[Bug c/99998] New: Unnecessary jump instruction

[dhowells at redhat dot com]

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

            Bug ID: 99998
           Summary: Unnecessary jump instruction
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dhowells at redhat dot com
  Target Milestone: ---

Created attachment 50539
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50539&action=edit
Test source

Using the Fedora 33 x86_64 compiler:
gcc version 10.2.1 20201125 (Red Hat 10.2.1-9) (GCC) 

Building the following (see also attached file):

typedef _Bool bool;
#define smp_rmb() __asm__ __volatile__("": : :"memory")
#define unlikely(x)     __builtin_expect(!!(x), 0)
enum { PG_uptodate = 2 };
struct page {
        unsigned long flags;
        unsigned long compound_head;    /* Bit zero is set */
};
static inline bool constant_test_bit(unsigned int nr, const void *addr)
{
        const unsigned int *p = (const unsigned int *)addr;
        return ((1UL << (nr & 31)) & (p[nr >> 5])) != 0;
}
static inline struct page *compound_head(struct page *page)
{
        unsigned long head = page->compound_head;

        if (unlikely(head & 1))
                return (struct page *) (head - 1);
        return page;
}
bool PageUptodate(struct page *page)
{
        bool ret;
        page = compound_head(page);
        ret = constant_test_bit(PG_uptodate, &page->flags);
        if (ret)
                smp_rmb();
        return ret;
}

with "gcc -Os" I get the following assembly excerpt:

PageUptodate:
.LFB2:
        .cfi_startproc
        movq    8(%rdi), %rax
        testb   $1, %al
        je      .L2
        leaq    -1(%rax), %rdi
.L2:
        movl    (%rdi), %eax
        shrq    $2, %rax
        andb    $1, %al
        je      .L1
.L1:
        ret
        .cfi_endproc

There's a superfluous jump, JE, at the end jumping to the next instruction with
label .L1.  This corresponds to the smp_rmb() which is just a compiler barrier.
 This also happens with other optimisation levels.

[Bug c/99998] Unnecessary jump instruction

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-04-12
           Keywords|                            |missed-optimization
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  The issue is that the RMB is conditionally skipped, so there
isn't anything we can do here unless we want to treat empty asm text
specially and thus essentially drop the asm.  I think the proper fix is
to make the smp_rmb unconditional in the source, or, if the kernel has
archs where the RMB isn't a no-op, add a conditional_smp_rmb (condition).

(insn 18 17 19 4 (set (reg:CCZ 17 flags)
        (compare:CCZ (reg:QI 88 [ <retval> ])
            (const_int 0 [0]))) "t.c":27:5 5 {*cmpqi_ccno_1}
     (nil))
(jump_insn 19 18 20 4 (set (pc)
        (if_then_else (eq (reg:CCZ 17 flags)
                (const_int 0 [0]))
            (label_ref:DI 26) 
            (pc))) "t.c":27:5 806 {*jcc}
     (int_list:REG_BR_PROB 536870916 (nil))
 -> 26)
;;  succ:       5 [50.0% (guessed)]  count:536870912 (estimated locally)
(FALLTHRU)
;;              6 [50.0% (guessed)]  count:536870912 (estimated locally)

;; basic block 5, loop depth 0, count 536870913 (estimated locally), maybe hot
;;  prev block 4, next block 6, flags: (RTL)
;;  pred:       4 [50.0% (guessed)]  count:536870912 (estimated locally)
(FALLTHRU)
(note 20 19 21 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(insn 21 20 26 5 (parallel [
            (asm_operands/v ("") ("") 0 []
                 []
                 [] t.c:28)
            (clobber (mem:BLK (scratch) [0  A8]))
            (clobber (reg:CC 17 flags))
        ]) "t.c":28:3 -1
     (nil))
;;  succ:       6 [always]  count:536870913 (estimated locally) (FALLTHRU)

;; basic block 6, loop depth 0, count 1073741824 (estimated locally), maybe hot
;;  prev block 5, next block 1, flags: (RTL)
;;  pred:       5 [always]  count:536870913 (estimated locally) (FALLTHRU)
;;              4 [50.0% (guessed)]  count:536870912 (estimated locally)
(code_label 26 21 29 6 1 (nil) [1 uses])
(note 29 26 27 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 27 29 28 6 (set (reg/i:QI 0 ax)
        (reg:QI 88 [ <retval> ])) "t.c":30:1 77 {*movqi_internal}
     (nil))
(insn 28 27 0 6 (use (reg/i:QI 0 ax)) "t.c":30:1 -1
     (nil))
;;  succ:       EXIT [always]  count:1073741824 (estimated locally) (FALLTHRU)


That said, this is WONTFIX I fear.

[Bug inline-asm/99998] Unnecessary jump instruction

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |WONTFIX
          Component|c                           |inline-asm
             Status|NEW                         |RESOLVED

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
GCC does not look into the inline-asm to see if it is empty or not.  Since the
inline-asm is marked as volatile, it cannot be made as unconditional either.
So closing as won't fix.