[Bug c/104839] New: ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[Bug c/104839] New: ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[vstinner at redhat dot com]

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

            Bug ID: 104839
           Summary: ppc64 regression of GCC 12: conditional long += long
                    becomes unconditional SUBFC
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vstinner at redhat dot com
  Target Milestone: ---

Created attachment 52583
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52583&action=edit
bug4.c: reproducer

When attached bug4.c is built on ppc64 with -O1, it returns 1 (bug). With -O0,
it returns 0 (ok), as expected.

$ gcc bug4.c -o bug -O1 && ./bug; echo $?
1
$ gcc bug4.c -o bug -O0 && ./bug; echo $?
0
$ uname -m
ppc64le

On x86-64, it returns 0 (ok) with -O0 and -02 as expected.

If you replace "long t_5 = i" with "int t_5 = i", it works as expected: a
conditional machine code is generated for the conditional "t_5 += shape" C
code.

--

The reproducer comes from this Cython bug:
https://bugzilla.redhat.com/show_bug.cgi?id=2045160

Cython compiles reproducer.pyx as ~23 000 lines of C code. The bug is in the
generated __pyx_pf_11reproducer2_get_schar() function.

Extract of the original code:
---
  __pyx_t_5 = __pyx_v_i;
  __pyx_t_6 = __pyx_v_j;
  __pyx_t_7 = -1;
  if (__pyx_t_5 < 0) {
    __pyx_t_5 += __pyx_v_buf.shape[0];
    if (unlikely(__pyx_t_5 < 0)) __pyx_t_7 = 0;
  } else if (unlikely(__pyx_t_5 >= __pyx_v_buf.shape[0])) __pyx_t_7 = 0;
  if (__pyx_t_6 < 0) {
    __pyx_t_6 += __pyx_v_buf.shape[1];
    if (unlikely(__pyx_t_6 < 0)) __pyx_t_7 = 1;
  } else if (unlikely(__pyx_t_6 >= __pyx_v_buf.shape[1])) __pyx_t_7 = 1;
  if (unlikely(__pyx_t_7 != -1)) {
    __Pyx_RaiseBufferIndexError(__pyx_t_7);
    __PYX_ERR(0, 140, __pyx_L1_error)
  }
---

It took me 2 days to simply the code and remove dependencies to Cython and
Python.

[Bug c/104839] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[vstinner at redhat dot com]

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

--- Comment #1 from Victor Stinner <vstinner at redhat dot com> ---
Thanks Miro Hrončok who identified the Cython regressiond and wrote a Cython
reproducer. Thanks Serge Guelton who helped me to write a shorter C reproducer.

[Bug target/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |12.0
            Summary|ppc64 regression of GCC 12: |[12 Regression] ppc64
                   |conditional long += long    |regression of GCC 12:
                   |becomes unconditional SUBFC |conditional long += long
                   |                            |becomes unconditional SUBFC
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |11.2.1
     Ever confirmed|0                           |1
   Target Milestone|---                         |12.0
           Keywords|                            |wrong-code
          Component|c                           |target
   Last reconfirmed|                            |2022-03-08
             Target|powerpc64le                 |powerpc64le-unknown-linux-g
                   |                            |nu

[Bug target/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The test fails with -O1 and passes with -O1 -fno-forward-propagate

Also, the test passes with -Og and fails with -Og -fif-conversion

[Bug target/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly adjusted testcase, so that it aborts on miscompilation:
__attribute__((noipa)) short
foo (void)
{
  return -1;
}

__attribute__((noipa)) int
bar (void)
{
  short i = foo ();
  if (i == -2)
    return 2;
  long k = i;
  int j = -1;
  volatile long s = 300;
  if (k < 0)
    {
      k += s;
      if (k < 0)
        j = 0;
    }
  else if (k >= s)
    j = 0;
  if (j != -1)
    return 1;
  return 0;
}

int
main ()
{
  if (bar () != 0)
    __builtin_abort ();
  return 0;
}

[Bug target/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-3252-gcad36f38576a6a781e3c62ab061c68f5b8dab13a

[Bug target/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This goes wrong during fwprop1.
In #c3 testcase at -O2 in bar we have before fwprop1:
(call_insn 6 2 7 2 (parallel [
            (set (reg:DI 3 3)
                (call (mem:SI (symbol_ref:DI ("foo") [flags 0x3] 
<function_decl 0x7fb75851b400 foo>) [0 foo S4 A8])
                    (const_int 0 [0])))
            (use (const_int 0 [0]))
            (clobber (reg:DI 96 lr))
        ]) "pr104839.c":10:13 755 {*call_value_localdi}
     (expr_list:REG_CALL_DECL (symbol_ref:DI ("foo") [flags 0x3] 
<function_decl 0x7fb75851b400 foo>)
        (nil))
    (expr_list (use (reg:DI 2 2))
        (nil)))
(insn 7 6 8 2 (set (reg:DI 126)
        (reg:DI 3 3)) "pr104839.c":10:13 670 {*movdi_internal64}
     (nil))
(insn 8 7 9 2 (set (reg/v:HI 119 [ i ])
        (subreg/s/u:HI (reg:DI 126) 0)) "pr104839.c":10:13 552
{*movhi_internal}
     (nil))
(insn 9 8 10 2 (set (reg:SI 127)
        (sign_extend:SI (reg/v:HI 119 [ i ]))) "pr104839.c":11:6 25
{*extendhisi2}
     (nil))
(insn 10 9 11 2 (set (reg:SI 128)
        (const_int -2 [0xfffffffffffffffe])) "pr104839.c":11:6 546
{*movsi_internal1}
     (nil))
(insn 11 10 12 2 (set (reg:CC 129)
        (compare:CC (reg:SI 127)
            (reg:SI 128))) "pr104839.c":11:6 785 {*cmpsi_signed}
     (nil))
which looks good to me, the function returns signed short and ppc64{,le} ABI
says that in that case the value is returned sign-extended into %r3.
subreg/s/u stands for promoted sign-extended.
But fwprop changes insns 8 to 11 into:
(insn 11 7 12 2 (set (reg:CC 129)
        (compare:CC (subreg/s/v:SI (reg:DI 126) 0)
            (const_int -2 [0xfffffffffffffffe]))) "pr104839.c":11:6 785
{*cmpsi_signed}
     (nil))
where subreg/s/v stands for promoted zero-extended.  That is wrong.

[Bug rtl-optimization/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
          Component|target                      |rtl-optimization

[Bug rtl-optimization/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:7ca24ae5701769fccabf8cca4cad6987cb2187c6

commit r12-7556-g7ca24ae5701769fccabf8cca4cad6987cb2187c6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 9 09:12:38 2022 +0100

    simplify-rtx: Fix up SUBREG_PROMOTED_SET arguments [PR104839]

    The following testcase is miscompiled on powerpc64le-linux at -O1 and
higher
    (except for -Og).  The bug was introduced in r12-3252-gcad36f38576a6a7
    which for SIGN_EXTEND from SUBREG_PROMOTED_SIGNED_P SUBREG used
    SUBREG_PROMOTED_SET (temp, 1) (but that makes temp
    SUBREG_PROMOTED_UNSIGNED_P because SRP_UNSIGNED is 1) and similarly the
    ZERO_EXTEND from SUBREG_PROMOTED_UNSIGNED_P SUBREG used
    SUBREG_PROMOTED_SET (temp, 0) (but that makes temp
    SUBREG_PROMOTED_SIGNED_P because SRP_SIGNED is 0).
    The following patch fixes that (swaps the 0s and 1s), but for better
    readability uses the SRP_* constants.
    rtl.h has:
    /* Valid for subregs which are SUBREG_PROMOTED_VAR_P().  In that case
       this gives the necessary extensions:
       0  - signed (SPR_SIGNED)
       1  - normal unsigned (SPR_UNSIGNED)
       2  - value is both sign and unsign extended for mode
            (SPR_SIGNED_AND_UNSIGNED).
       -1 - pointer unsigned, which most often can be handled like unsigned
            extension, except for generating instructions where we need to
            emit special code (ptr_extend insns) on some architectures
            (SPR_POINTER). */
    The expr.c change in the same commit looks ok to me (passes unsignedp
    to SUBREG_PROMOTED_SET, so 0 for signed, 1 for unsigned).

    2022-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104839
            * simplify-rtx.cc (simplify_unary_operation_1) <case SIGN_EXTEND>:
            Use SRP_SIGNED instead of incorrect 1 in SUBREG_PROMOTED_SET.
            (simplify_unary_operation_1) <case ZERO_EXTEND>: Use SRP_UNSIGNED
            instead of incorrect 0 in SUBREG_PROMOTED_SET.

            * gcc.c-torture/execute/pr104839.c: New test.

[Bug rtl-optimization/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug rtl-optimization/104839] [12 Regression] ppc64 regression of GCC 12: conditional long += long becomes unconditional SUBFC

[vstinner at redhat dot com]

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

--- Comment #8 from Victor Stinner <vstinner at redhat dot com> ---
> Fixed.

Thank you! That was quick!