[Bug c/101642] New: ice in decompose, at wide-int.h:984

[Bug c/101642] New: ice in decompose, at wide-int.h:984

[dcb314 at hotmail dot com]

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

            Bug ID: 101642
           Summary: ice in decompose, at wide-int.h:984
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

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

The attached C code does this with recent gcc:

$ ../results/bin/gcc -c -O2 bug740.c 
during GIMPLE pass: fre
netflow_v1.c: In function ‘Process_v1’:
netflow_v1.c:507:1: internal compiler error: in decompose, at wide-int.h:984
0x139979c wi::int_traits<generic_wide_int<wide_int_storage> >::decompose(long*, 
unsigned int, generic_wide_int<wide_int_storage> const&)
        ../../trunk.git/gcc/wide-int.h:984
0x139979c wide_int_ref_storage<true,
false>::wide_int_ref_storage<generic_wide_i
nt<wide_int_storage> >(generic_wide_int<wide_int_storage> const&, unsigned int)
        ../../trunk.git/gcc/wide-int.h:1034
0x139979c generic_wide_int<wide_int_ref_storage<true, false>
>::generic_wide_int
<generic_wide_int<wide_int_storage> >(generic_wide_int<wide_int_storage>
const&,
 unsigned int)
        ../../trunk.git/gcc/wide-int.h:790

I will have my usual go at reducing the code and trying to spot a
range of git hashes where the problem first occurs.

[Bug c/101642] ice in decompose, at wide-int.h:984

[dcb314 at hotmail dot com]

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

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---
Problem first seems to occur sometime between git hash ead235f60139edc6,
dated 20210724 and git hash fcc7c6369f7fbf29, dated today.

[Bug c/101642] ice in decompose, at wide-int.h:984

[dcb314 at hotmail dot com]

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

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
Reduced code seems to be

short __bswap_16___bsx, Process_v1___trans_tmp_1;
int Process_v1_common_record;
char Process_v1_s2;
void Process_v1(void) {
  Process_v1___trans_tmp_1 = __builtin_bswap16(__bswap_16___bsx);
  if (Process_v1___trans_tmp_1)
    Process_v1_s2 = Process_v1_common_record;
}

[Bug c/101642] ice in decompose, at wide-int.h:984

[dcb314 at hotmail dot com]

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

--- Comment #3 from David Binderman <dcb314 at hotmail dot com> ---
In a git bisect, trying cf5f544227f16b63

[Bug c/101642] ice in decompose, at wide-int.h:984

[dcb314 at hotmail dot com]

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

--- Comment #4 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to David Binderman from comment #3)
> In a git bisect, trying cf5f544227f16b63

Seems bad, so trying 1ab2270036dc0f2a.

[Bug c/101642] ice in decompose, at wide-int.h:984

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-2516-gcf5f544227f16b63e224529190329eb0edca791c

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|ice in decompose, at        |[12 Regression] ice in
                   |wide-int.h:984              |decompose, at
                   |                            |wide-int.h:984
   Target Milestone|---                         |12.0
                 CC|                            |sayle at gcc dot gnu.org
          Component|c                           |middle-end

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[zsojka at seznam dot cz]

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

Zdenek Sojka <zsojka at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zsojka at seznam dot cz

--- Comment #6 from Zdenek Sojka <zsojka at seznam dot cz> ---
Even simpler testcase failing at -O1:
$ cat testcase.c
int x;

void
foo (void)
{
  __builtin_bswap16 (x) ? : x;
}
$ x86_64-pc-linux-gnu-gcc -O testcase.c 
during GIMPLE pass: fre
testcase.c: In function 'foo':
testcase.c:7:1: internal compiler error: in decompose, at wide-int.h:984
    7 | }
      | ^
0x7f7b80 wi::int_traits<generic_wide_int<wide_int_ref_storage<false, false> >
>::decompose(long*, unsigned int, generic_wide_int<wide_int_ref_storage<false,
false> > const&)
...

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think better testcase would be
int x;

unsigned short
foo (void)
{
  return __builtin_bswap16 (x) ? : x;
}
because otherwise you're asking for it to be DCEd.

The problem is that for __builtin_bswap16 like for many other builtins with 8
or 16-bit argument(s) the operand is promoted to int.
So, many of the match.pd rules just don't trigger for it at all, like:
 (simplify
  (bswap (bswap @0))
  @0)
 (simplify
  (bswap (bit_not (bswap @0)))
  (bit_not @0))
 (for bitop (bit_xor bit_ior bit_and)
  (simplify
   (bswap (bitop:c (bswap @0) @1))
   (bitop @0 (bswap @1))))
because there are casts it doesn't take into account for bswap16, others are
just incorrect, like:
 (for cmp (eq ne)
  (simplify
   (cmp (bswap @0) (bswap @1))
   (cmp @0 @1))
- it needs to convert @0 and @1 to the type of one of the bswaps, otherwise it
could be comparing even the upper bits, and others are both incorrect and cause
ICE, like
  (simplify
    (cmp (bswap @0) INTEGER_CST@1)
   (cmp @0 (bswap @1))))
Let me deal with just those now and the rest that just don't match can be dealt
with later on.

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[zsojka at seznam dot cz]

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

--- Comment #8 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Jakub Jelinek from comment #7)
> I think better testcase would be
> int x;
> 
> unsigned short
> foo (void)
> {
>   return __builtin_bswap16 (x) ? : x;
> }
> because otherwise you're asking for it to be DCEd.
> 

Indeed, but when I tried this, it didn't ICE any more. (but I didn't explore
this any further)

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
With unsigned short return it ICEs, with int return it doesn't.

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-07-27
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51212
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51212&action=edit
gcc12-pr101642.patch

Untested fix.

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roger at nextmovesoftware dot com

--- Comment #11 from Roger Sayle <roger at nextmovesoftware dot com> ---
Hi Jakub,
Sorry for the inconvenience.
I've just come up with a near-identical fix.  I think it's safe to strip the
(symmetric) bswaps in the first transformation, but I'm now paranoid enough to
explicitly convert both operands of the cmp to the same comparison type in the
second transformation.
-   (cmp (bswap @0) INTEGER_CST@1)
-   (cmp @0 (bswap @1))))
+   (cmp (bswap@0 @1) INTEGER_CST@2)
+   (with { tree t = TREE_TYPE(@0); }
+    (cmp (convert:t @1) (convert:t (bswap @2))))))

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 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:b4fc4df94f703e3b191f503ebe7186500e45d081

commit r12-2575-gb4fc4df94f703e3b191f503ebe7186500e45d081
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jul 28 18:41:50 2021 +0200

    match.pd: Fix up recent __builtin_bswap16 simplifications [PR101642]

    The following testcase ICEs.  The problem is that for __builtin_bswap16
    (and only that, others are fine) the argument of the builtin is promoted
    to int while the patterns assume it is not and is the same as that of
    the return type.
    For the bswap simplifications before these new ones it just means we
    fail to optimize stuff like __builtin_bswap16 (__builtin_bswap16 (x))
    because there are casts in between, but the last one, equality comparison
    of __builtin_bswap16 with integer constant results in ICE, because
    we create comparison with incompatible types of the operands, and the
    other might be fine because usually we bit and the operand before
promoting,
    but I think it is too dangerous to rely on it, one day we find out that
    because it is operand to such a built in, we can throw away any changes
    that affect the upper bits and all of sudden it would misbehave.

    So, this patch introduces converts that shouldn't do anything for
    bswap{32,64,128} and should fix these issues for bswap16.

    2021-07-28  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/101642
            * match.pd (bswap16 (x) == bswap16 (y)): Cast both operands
            to type of bswap16 for comparison.
            (bswap16 (x) == cst): Cast bswap16 operand to type of cst.

            * gcc.c-torture/compile/pr101642.c: New test.

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[jakub at gcc dot gnu.org]

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

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

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

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

[Bug middle-end/101642] [12 Regression] ice in decompose, at wide-int.h:984

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:

https://gcc.gnu.org/g:f9fcf754825a1e01033336f84c18690aaa971a6f

commit r12-2657-gf9fcf754825a1e01033336f84c18690aaa971a6f
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Mon Aug 2 13:27:53 2021 +0100

    Optimize x ? bswap(x) : 0 in tree-ssa-phiopt

    Many thanks again to Jakub Jelinek for a speedy fix for PR 101642.
    Interestingly, that test case "bswap16(x) ? : x" also reveals a
    missed optimization opportunity.  The resulting "x ? bswap(x) : 0"
    can be further simplified to just bswap(x).

    Conveniently, tree-ssa-phiopt.c already recognizes/optimizes the
    related "x ? popcount(x) : 0", so this patch simply makes that
    transformation make general, additionally handling bswap, parity,
    ffs and clrsb.  All of the required infrastructure is already
    present thanks to Jakub previously adding support for clz/ctz.
    To reflect this generalization, the name of the function is changed
    from cond_removal_in_popcount_clz_ctz_pattern to the hopefully
    equally descriptive cond_removal_in_builtin_zero_pattern.

    2021-08-02  Roger Sayle  <roger@nextmovesoftware.com>

    gcc/ChangeLog
            * tree-ssa-phiopt.c (cond_removal_in_builtin_zero_pattern):
            Renamed from cond_removal_in_popcount_clz_ctz_pattern.
            Add support for BSWAP, FFS, PARITY and CLRSB builtins.
            (tree_ssa_phiop_worker): Update call to function above.

    gcc/testsuite/ChangeLog
            * gcc.dg/tree-ssa/phi-opt-25.c: New test case.