[Bug tree-optimization/100672] New: wrong code with vector shift and unary minus

[Bug tree-optimization/100672] New: wrong code with vector shift and unary minus

[zsojka at seznam dot cz]

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

            Bug ID: 100672
           Summary: wrong code with vector shift and unary minus
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu

Created attachment 50843
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50843&action=edit
reduced testcase

Output:
$ x86_64-pc-linux-gnu-gcc testcase.c -Wno-psabi
$ ./a.out 
Aborted

Seems to happen at any optimization level, and for any 32bit or 64bit target.

Rewriting the expression as "(-v >> 1)" results in the expected output.

$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r12-905-20210519101846-ga73a5af281c-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--disable-bootstrap --with-cloog --with-ppl --with-isl
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r12-905-20210519101846-ga73a5af281c-checking-yes-rtl-df-extra-nobootstrap-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.0 20210519 (experimental) (GCC)

[Bug tree-optimization/100672] wrong code with vector shift and unary minus

[zsojka at seznam dot cz]

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

Zdenek Sojka <zsojka at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |10.3.1, 11.1.1, 12.0,
                   |                            |5.5.0, 6.5.0, 7.5.0, 8.4.0,
                   |                            |9.3.1

--- Comment #1 from Zdenek Sojka <zsojka at seznam dot cz> ---
Adding 8.4.0 as "Known to fail", since 8.5.0 cannot be entered:
The value "8.5.0" used in the "Known to fail" field is not a valid version for
the "gcc" product.

[Bug tree-optimization/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Maybe sth is wrong with the testcase?  clang also results in an abort.  Isn't
right-shift of negative values undefined?

.original dumps

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


{
  return VIEW_CONVERT_EXPR<V>(VIEW_CONVERT_EXPR<vector(4) signed long>(v) >>
1);
}

where the negate is dopped somehow.

[Bug tree-optimization/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-05-19

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
We have in negate_expr/negate_expr_p

    case RSHIFT_EXPR:
      /* Optimize -((int)x >> 31) into (unsigned)x >> 31 for int.  */
      if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST)
        {
          tree op1 = TREE_OPERAND (t, 1);
          if (wi::to_wide (op1) == TYPE_PRECISION (type) - 1)
            {
              tree ntype = TYPE_UNSIGNED (type)
                           ? signed_type_for (type)
                           : unsigned_type_for (type);
              tree temp = fold_convert_loc (loc, ntype, TREE_OPERAND (t, 0));
              temp = fold_build2_loc (loc, RSHIFT_EXPR, ntype, temp, op1);
              return fold_convert_loc (loc, type, temp);
            }
        }

note the use of TYPE_PRECISION on a vector type...

[Bug tree-optimization/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 419117dca3f..ef0d15a289d 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -512,7 +512,7 @@ negate_expr_p (tree t)
       if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST)
        {
          tree op1 = TREE_OPERAND (t, 1);
-         if (wi::to_wide (op1) == TYPE_PRECISION (type) - 1)
+         if (wi::to_wide (op1) == element_precision (type) - 1)
            return true;
        }
       break;
@@ -705,7 +705,7 @@ fold_negate_expr_1 (location_t loc, tree t)
       if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST)
        {
          tree op1 = TREE_OPERAND (t, 1);
-         if (wi::to_wide (op1) == TYPE_PRECISION (type) - 1)
+         if (wi::to_wide (op1) == element_precision (type) - 1)
            {
              tree ntype = TYPE_UNSIGNED (type)
                           ? signed_type_for (type)

fixes that, retaining the negate, but it's still broken somewhere.

[Bug tree-optimization/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, don't you want a signed vector component?  You're getting a logical right
shift as written.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
          Component|tree-optimization           |middle-end
             Status|WAITING                     |ASSIGNED

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
typedef long long __attribute__((__vector_size__ (32))) V;

V
foo (V v)
{
  return -(v >> 1);
}

int
main (void)
{
  V v = foo ((V) { -2, -4, -6, -8 });
  if (v[0] != 1 || v[1] != 2 || v[2] != 3 || v[3] != 4)
    __builtin_abort ();
  return 0;
}


fails before my patch and succeeds afterwards.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[zsojka at seznam dot cz]

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

--- Comment #7 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Richard Biener from comment #2)
> Maybe sth is wrong with the testcase?  clang also results in an abort.  Isn't
> right-shift of negative values undefined?
> 

Thank you for having a look.

I believe the behavior is fully defined. The vectors are unsigned, and the
conversion to unsigned is done by adding 2**64; this behaves the same:

$ cat testcase.c
typedef unsigned long long __attribute__((__vector_size__ (32))) V;

V
foo (V v)
{
  return -(v >> 1);
}

int
main (void)
{
  V v = foo ((V) { 0xfffffffffffffffe, 0xfffffffffffffffc,
                   0xfffffffffffffffa, 0xfffffffffffffff8 });
  if (v[0] != 1 || v[1] != 2 || v[2] != 3 || v[3] != 4)
    __builtin_abort ();
  return 0;
}

Maybe clang behaves the same since the behavior was the same in the moment when
clang was forked.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Zdenek Sojka from comment #7)
> (In reply to Richard Biener from comment #2)
> > Maybe sth is wrong with the testcase?  clang also results in an abort.  Isn't
> > right-shift of negative values undefined?
> > 
> 
> Thank you for having a look.
> 
> I believe the behavior is fully defined. The vectors are unsigned, and the
> conversion to unsigned is done by adding 2**64; this behaves the same:
> 
> $ cat testcase.c
> typedef unsigned long long __attribute__((__vector_size__ (32))) V;
> 
> V
> foo (V v)
> {
>   return -(v >> 1);

but this is a logical right shift, thus gives 0x7ff...e, ...
and negating that doesn't yield 1.

> }
> 
> int
> main (void)
> {
>   V v = foo ((V) { 0xfffffffffffffffe, 0xfffffffffffffffc,
>                    0xfffffffffffffffa, 0xfffffffffffffff8 });
>   if (v[0] != 1 || v[1] != 2 || v[2] != 3 || v[3] != 4)
>     __builtin_abort ();
>   return 0;
> }
> 
> Maybe clang behaves the same since the behavior was the same in the moment
> when clang was forked.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[zsojka at seznam dot cz]

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

--- Comment #9 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Richard Biener from comment #8)
> (In reply to Zdenek Sojka from comment #7)
> > (In reply to Richard Biener from comment #2)
> > > Maybe sth is wrong with the testcase?  clang also results in an abort.  Isn't
> > > right-shift of negative values undefined?
> > > 
> > 
> > Thank you for having a look.
> > 
> > I believe the behavior is fully defined. The vectors are unsigned, and the
> > conversion to unsigned is done by adding 2**64; this behaves the same:
> > 
> > $ cat testcase.c
> > typedef unsigned long long __attribute__((__vector_size__ (32))) V;
> > 
> > V
> > foo (V v)
> > {
> >   return -(v >> 1);
> 
> but this is a logical right shift, thus gives 0x7ff...e, ...
> and negating that doesn't yield 1.
> 

Shame on me. You are very right.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[zsojka at seznam dot cz]

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

--- Comment #10 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Zdenek Sojka from comment #9)
> (In reply to Richard Biener from comment #8)
> > (In reply to Zdenek Sojka from comment #7)
> > > (In reply to Richard Biener from comment #2)
> > > > Maybe sth is wrong with the testcase?  clang also results in an abort.  Isn't
> > > > right-shift of negative values undefined?
> > > > 
> > > 
> > > Thank you for having a look.
> > > 
> > > I believe the behavior is fully defined. The vectors are unsigned, and the
> > > conversion to unsigned is done by adding 2**64; this behaves the same:
> > > 
> > > $ cat testcase.c
> > > typedef unsigned long long __attribute__((__vector_size__ (32))) V;
> > > 
> > > V
> > > foo (V v)
> > > {
> > >   return -(v >> 1);
> > 
> > but this is a logical right shift, thus gives 0x7ff...e, ...
> > and negating that doesn't yield 1.
> > 
> 
> Shame on me. You are very right.

I did the mistake while reducing the testcase.
The original testcase had positive arguments:

$ cat testcase.c
typedef unsigned long long __attribute__((__vector_size__ (32))) V;

V
foo (V v)
{
  return -(v >> 1);
}

int
main (void)
{
  V v = foo ((V) { 2, 4, 6, 8 });
  if (v[0] != 0xffffffffffffffff || v[1] != 0xfffffffffffffffe ||
      v[2] != 0xfffffffffffffffd || v[3] != 0xfffffffffffffffc)
    __builtin_abort ();
  return 0;
}


I wanted to prevent the long 0xfffff... constants, but broke the testcase in he
process. Thank you for correcting me.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:8d51039cb7c807ed84ff7df5416a1e3ba07a5e63

commit r12-913-g8d51039cb7c807ed84ff7df5416a1e3ba07a5e63
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 19 13:35:07 2021 +0200

    middle-end/100672 - fix bogus right shift folding

    This fixes the bogus use of TYPE_PRECISION on vector types
    from optimizing -((int)x >> 31) into (unsigned)x >> 31.

    2021-05-19  Richard Biener  <rguenther@suse.de>

            PR middle-end/100672
            * fold-const.c (fold_negate_expr_1): Use element_precision.
            (negate_expr_p): Likewise.

            * gcc.dg/torture/pr100672.c: New testcase.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|12.0                        |
      Known to work|                            |12.0

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug middle-end/100672] wrong code with vector shift and unary minus

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r11-8652-ge0dae9c92aa248c858f682f03ee1b16471297718
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 19 13:35:07 2021 +0200

    middle-end/100672 - fix bogus right shift folding

    This fixes the bogus use of TYPE_PRECISION on vector types
    from optimizing -((int)x >> 31) into (unsigned)x >> 31.

    2021-05-19  Richard Biener  <rguenther@suse.de>

            PR middle-end/100672
            * fold-const.c (fold_negate_expr_1): Use element_precision.
            (negate_expr_p): Likewise.

            * gcc.dg/torture/pr100672.c: New testcase.

    (cherry picked from commit 8d51039cb7c807ed84ff7df5416a1e3ba07a5e63)

[Bug middle-end/100672] wrong code with vector shift and unary minus

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r10-10261-ge8311f59c6e2fa1f5aacc3da5e95739886ebdee7
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 19 13:35:07 2021 +0200

    middle-end/100672 - fix bogus right shift folding

    This fixes the bogus use of TYPE_PRECISION on vector types
    from optimizing -((int)x >> 31) into (unsigned)x >> 31.

    2021-05-19  Richard Biener  <rguenther@suse.de>

            PR middle-end/100672
            * fold-const.c (fold_negate_expr_1): Use element_precision.
            (negate_expr_p): Likewise.

            * gcc.dg/torture/pr100672.c: New testcase.

    (cherry picked from commit 8d51039cb7c807ed84ff7df5416a1e3ba07a5e63)

[Bug middle-end/100672] wrong code with vector shift and unary minus

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r9-9820-gcc589cc7923b048b2722980cac69463b6f2e1941
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 19 13:35:07 2021 +0200

    middle-end/100672 - fix bogus right shift folding

    This fixes the bogus use of TYPE_PRECISION on vector types
    from optimizing -((int)x >> 31) into (unsigned)x >> 31.

    2021-05-19  Richard Biener  <rguenther@suse.de>

            PR middle-end/100672
            * fold-const.c (fold_negate_expr_1): Use element_precision.
            (negate_expr_p): Likewise.

            * gcc.dg/torture/pr100672.c: New testcase.

    (cherry picked from commit 8d51039cb7c807ed84ff7df5416a1e3ba07a5e63)

[Bug middle-end/100672] wrong code with vector shift and unary minus

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
      Known to fail|9.3.1                       |9.3.0
      Known to work|                            |9.3.1

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.