[Bug tree-optimization/106786] New: Regression in cmp+sbb

[Bug tree-optimization/106786] New: Regression in cmp+sbb

[chfast at gmail dot com]

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

            Bug ID: 106786
           Summary: Regression in cmp+sbb
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chfast at gmail dot com
  Target Milestone: ---

I noticed a regression when using the builtin for sbb instruction
(__builtin_ia32_sbb_u64).

typedef unsigned long long u64;

struct R {
    u64 value;
    bool carry;
};

inline R subc(u64 x, u64 y, bool carry) noexcept {
    u64 d;
    const u64 carryout = __builtin_ia32_sbb_u64(carry, x, y, &d);
    return {d, carryout != 0};
}

bool bad(u64 x, u64 y) {
    const R z = subc(x, y, false);
    R a = subc(x, y, z.carry);
    return a.carry;
}

https://godbolt.org/z/f41KKe19q

The expected assembly is
        cmp     rdi, rsi
        sbb     rdi, rsi

But GCC 12.2.0 and trunk produces
        cmp     rdi, rsi
        setb    al
        movzx   eax, al
        add     al, -1
        sbb     rdi, rsi

The regression is in 12.2.0, the 11.3.0 optimizes properly.

There are simple changes which will bring back the expected optimization:
- change `const R z` to `R z`,
- change `bool carry` to `u64 carry`.

This may be related to calling convention / ABI because I noticed in one of the
tree optimization outputs for 12.2.0 that the `bool carry` is forced to be in
memory: `MEM <unsigned char> [(struct R *)&z + 8B]`.

https://godbolt.org/z/7zh7GxraK

[Bug tree-optimization/106786] Regression in cmp+sbb

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |tree-optimization

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
-fno-tree-sra fixes the issue ...

[Bug tree-optimization/106786] [12/13 Regression] Regression in cmp+sbb

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-08-31
           Keywords|                            |needs-bisection
             Status|UNCONFIRMED                 |NEW
            Summary|Regression in cmp+sbb       |[12/13 Regression]
                   |                            |Regression in cmp+sbb
   Target Milestone|---                         |12.3
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The difference for esra IR is:
Working (11):
  z.value = SR.6_6;
  MEM <unsigned char> [(struct R *)&z + 8B] = SR.7_7;
  z$8_32 = SR.7_7;
  _1 = VIEW_CONVERT_EXPR<bool>(z$8_32);

Not working (12):
  z.value = SR.5_6;
  MEM <unsigned char> [(struct R *)&z + 8B] = SR.6_7;
  _1 = z.carry;

The IR is the similar (the only difference is the decl #) before esra:

  D.2444.carry = _19;
  d ={v} {CLOBBER(eol)};
  z = D.2444;
  _1 = z.carry;
  _2 = (int) _1;
  _12 = (int) _1;

[Bug tree-optimization/106786] [12/13 Regression] SRA regression causes extra instructions sometimes

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
             Blocks|                            |100453
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The difference is (before SRA) good vs. bad:

+Rejected (2400): not aggregate: x
+Rejected (2401): not aggregate: y
+Candidate (2452): D.2452
+Rejected (2451): not aggregate: d
+Candidate (2447): D.2447
+Rejected (2446): not aggregate: d
+Rejected (2445): not aggregate: carry
+Candidate (2430): a
+Candidate (2404): z
+! Disqualifying z - Encountered a store to a read-only decl.
+Will attempt to totally scalarize D.2447 (UID: 2447): 
+Will attempt to totally scalarize D.2452 (UID: 2452): 
 Changing the type of a replacement for a offset: 64, size: 8  to an integer.
-Created a replacement for a offset: 64, size: 8: a$8D.2422
+Created a replacement for a offset: 64, size: 8: a$8D.2453

so it looks like SRA is confused by 'z' being declared const?

Thus likely caused by r12-1529-gd7deee423f993b

diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index 1a3e12f18cc..1f4e5987292 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -922,12 +922,6 @@ create_access (tree expr, gimple *stmt, bool write)
   if (!DECL_P (base) || !bitmap_bit_p (candidate_bitmap, DECL_UID (base)))
     return NULL;

-  if (write && TREE_READONLY (base))
-    {
-      disqualify_candidate (base, "Encountered a store to a read-only decl.");
-      return NULL;
-    }
-
   HOST_WIDE_INT offset, size, max_size;
   if (!poffset.is_constant (&offset)
       || !psize.is_constant (&size)

fixes the regression (leaving the rest of the checks in place, but not sure
how safe that is).

Martin?


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100453
[Bug 100453] [12 Regression] wrong code at -O1 and above since r12-434

[Bug tree-optimization/106786] [12/13 Regression] SRA regression causes extra instructions sometimes

[chfast at gmail dot com]

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

--- Comment #4 from Paweł Bylica <chfast at gmail dot com> ---
Any update on this? I've identified some other similar cases where this hurting
the performance.

[Bug tree-optimization/106786] [12/13 Regression] SRA regression causes extra instructions sometimes since r12-1529-gd7deee423f993bee

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] SRA      |[12/13 Regression] SRA
                   |regression causes extra     |regression causes extra
                   |instructions sometimes      |instructions sometimes
                   |                            |since
                   |                            |r12-1529-gd7deee423f993bee
           Keywords|needs-bisection             |
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-1529-gd7deee423f993bee.

[Bug tree-optimization/106786] [12/13/14 Regression] SRA regression causes extra instructions sometimes since r12-1529-gd7deee423f993bee

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.