[Bug c/108370] New: gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[Bug c/108370] New: gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[dhowells at redhat dot com]

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

            Bug ID: 108370
           Summary: gcc doesn't merge bitwise-AND if an explicit
                    comparison against 0 is given
           Product: gcc
           Version: 12.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 54245
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54245&action=edit
Demo code

If gcc sees a couple of calls to an inline function that does a bitwise-AND and
returns whether the result was zero or non-zero (e.g. a flag check helper), gcc
cannot merge them if the result of the AND is explicitly compared against 0,
even if the function's return type is a bool (which would do that anyway).  For
example:

   static inline bool bio_flagged(struct bio *bio, unsigned int bit)
   {
        return (bio->bi_flags & (1U << bit)) != 0;
   }

   void bio_release_pages(struct bio *bio, bool mark_dirty)
   {
        if (bio_flagged(bio, BIO_PAGE_REFFED) ||
            bio_flagged(bio, BIO_PAGE_PINNED))
                __bio_release_pages(bio, mark_dirty);
   }

compiles bio_release_pages() to:

   0:   66 8b 07                mov    (%rdi),%ax
   3:   a8 01                   test   $0x1,%al
   5:   75 04                   jne    b <bio_release_pages+0xb>
   7:   a8 02                   test   $0x2,%al
   9:   74 09                   je     14 <bio_release_pages+0x14>
   b:   40 0f b6 f6             movzbl %sil,%esi
   f:   e9 00 00 00 00          jmp    14 <bio_release_pages+0x14>
  14:   c3                      ret    

but:

   static inline bool bio_flagged(struct bio *bio, unsigned int bit)
   {
        return bio->bi_flags & (1U << bit);
   }

gives:

   0:   f6 07 03                testb  $0x3,(%rdi)
   3:   74 09                   je     e <bio_release_pages+0xe>
   5:   40 0f b6 f6             movzbl %sil,%esi
   9:   e9 00 00 00 00          jmp    e <bio_release_pages+0xe>
   e:   c3                      ret    

Possibly the comparison against 0 could be optimised away.

I've attached some demo code that can be compiled with one of:

gcc -Os -c gcc-bool-demo.c
gcc -Os -c gcc-bool-demo.c -Dfix

The gcc I used above is the Fedora 37 system compiler:

gcc-12.2.1-4.fc37.x86_64
binutils-2.38-25.fc37.x86_64

but similar results can be seen with the Fedora arm cross-compiler:

   0:   e1d030b0        ldrh    r3, [r0]
   4:   e3130001        tst     r3, #1
   8:   1a000001        bne     14 <bio_release_pages+0x14>
   c:   e3130002        tst     r3, #2
  10:   012fff1e        bxeq    lr
  14:   eafffffe        b       0 <__bio_release_pages>

vs

   0:   e1d030b0        ldrh    r3, [r0]
   4:   e3130003        tst     r3, #3
   8:   012fff1e        bxeq    lr
   c:   eafffffe        b       0 <__bio_release_pages>

[Bug c/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-01-11
     Ever confirmed|0                           |1
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
The good case is handled by ifcombine:

optimizing bits or bits test to _5 & T != 0
with temporary T = 2 | 1
Merging blocks 2 and 3

  <bb 2> [local count: 1073741824]:
  _5 = bio_4(D)->bi_flags;
  _8 = _5 & 1;
  if (_8 != 0)
    goto <bb 4>; [33.00%]
  else
    goto <bb 3>; [67.00%]

  <bb 3> [local count: 719407025]:
  _9 = _5 & 2;
  if (_9 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 714038313]:
  _1 = (int) mark_dirty_6(D);
  __bio_release_pages (bio_4(D), _1);

but the bad case is not handled:

  <bb 2> [local count: 1073741824]:
  _6 = bio_4(D)->bi_flags;
  _5 = (unsigned int) _6;
  _9 = (_Bool) _6;
  if (_9 != 0)
    goto <bb 4>; [33.00%]
  else
    goto <bb 3>; [67.00%]

  <bb 3> [local count: 719407025]:
  _10 = _5 >> 1;
  _11 = (_Bool) _10;
  if (_11 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 714038313]:
  _1 = (int) mark_dirty_7(D);
  __bio_release_pages (bio_4(D), _1);

it looks like some premature optimization triggered (not) there.

.original is (good)

;; Function bio_flagged (null)
;; enabled by -tree-original


{
  return ((unsigned int) bio->bi_flags & 1 << bit) != 0;
}

vs (bad)

;; Function bio_flagged (null)
;; enabled by -tree-original


{
  return ((unsigned int) bio->bi_flags >> bit & 1) != 0;
}

I suppose one variant is folded after the promotion to bool and one
before only.

[Bug c/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
ifcombine seems to assume that

     D.1987_7 = op0 & 1;
     if (D.1987_7 != 0)

is canonical but we see

  _9 = (_Bool) _6;
  if (_9 != 0)

instead.  That's already the form introduced by inlining from

_Bool bio_flagged (struct bio * bio, unsigned int bit)
{
  short unsigned int _1;
  unsigned int _2;
  unsigned int _3;
  unsigned int _4;
  _Bool _8;

  <bb 2> :
  _1 = bio_6(D)->bi_flags;
  _2 = (unsigned int) _1;
  _3 = _2 >> bit_7(D);
  _4 = _3 & 1;
  _8 = _4 != 0;
  return _8;

and

  <bb 2> :
  _1 = bio_flagged (bio_7(D), 0);
  if (_1 != 0)
    goto <bb 4>; [INV]
  else
    goto <bb 3>; [INV]

[Bug c/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[dhowells at redhat dot com]

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

--- Comment #3 from dhowells at redhat dot com <dhowells at redhat dot com> ---
We don't want to do:

   return ((unsigned int) bio->bi_flags >> bit & 1) != 0;

if we can avoid it as "bit" is usually constant - though I'm guessing the
optimiser should handle that?

[Bug middle-end/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
          Component|c                           |middle-end

[Bug middle-end/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Mine.

[Bug middle-end/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
  _10 = _9 >> 1;
  _11 = (bool) _10;
  if (_11 != 0)


Should just be optimized to:
_t = _9 & 1
if (_t != 0)

Let me add that to match.

[Bug middle-end/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
>   _10 = _9 >> 1;
>   _11 = (bool) _10;
>   if (_11 != 0)
> 
> 
> Should just be optimized to:
> _t = _9 & 1
> if (_t != 0)
> 
> Let me add that to match.

We do handle:
/* Fold ((X << C1) & C2) cmp C3 into (X & (C2 >> C1)) cmp (C3 >> C1)
        ((X >> C1) & C2) cmp C3 into (X & (C2 << C1)) cmp (C3 << C1).  */

[Bug middle-end/108370] gcc doesn't merge bitwise-AND if an explicit comparison against 0 is given

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55914
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55914&action=edit
Patch

Combined with the patch for PR 109960. We are able to optimize this correctly:
  _5 = bio_4(D)->bi_flags;
  _8 = _5 & 3;
  if (_8 != 0)
    goto <bb 3>; [66.50%]
  else
    goto <bb 4>; [33.50%]

  <bb 3> [local count: 714038312]:
  _1 = (int) mark_dirty_6(D);
  __bio_release_pages (bio_4(D), _1); [tail call]