public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2
@ 2020-10-21 16:27 zsojka at seznam dot cz
  2020-10-21 16:37 ` [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394 jakub at gcc dot gnu.org
                   ` (24 more replies)
  0 siblings, 25 replies; 26+ messages in thread
From: zsojka at seznam dot cz @ 2020-10-21 16:27 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 97521
           Summary: [11 Regression] wrong code with -mno-sse2
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu

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

Output:
$ x86_64-pc-linux-gnu-gcc -O -mno-sse2 testcase.c
$ ./a.out 
Aborted

$ 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-r11-4185-20201021114306-gd9409301387-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/11.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
--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-r11-4185-20201021114306-gd9409301387-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.0 20201021 (experimental) (GCC)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
@ 2020-10-21 16:37 ` jakub at gcc dot gnu.org
  2020-10-21 16:56 ` jakub at gcc dot gnu.org
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-21 16:37 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |11.0
            Summary|[11 Regression] wrong code  |[11 Regression] wrong code
                   |with -mno-sse2              |with -mno-sse2 since
                   |                            |r11-3394
   Last reconfirmed|                            |2020-10-21

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r11-3394-gbc909324bda71543add2229adfa59d8daff5f0db

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
  2020-10-21 16:37 ` [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394 jakub at gcc dot gnu.org
@ 2020-10-21 16:56 ` jakub at gcc dot gnu.org
  2020-10-21 17:07 ` jakub at gcc dot gnu.org
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-21 16:56 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It seems that
  vector(2) <signed-boolean:64> _1;
  vector(2) <signed-boolean:64> _2;
...
  _1 = _2 & { 0, -1 };
remains in the IL after that change, which results in expansion of TImode AND,
but the second operand of that is (const_int 2 [0x2]) and not the expected
0xffffffffffffffff0000000000000000 constant.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
  2020-10-21 16:37 ` [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394 jakub at gcc dot gnu.org
  2020-10-21 16:56 ` jakub at gcc dot gnu.org
@ 2020-10-21 17:07 ` jakub at gcc dot gnu.org
  2020-10-21 18:33 ` marxin at gcc dot gnu.org
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-21 17:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And we do that because:
    case VECTOR_CST:
      {
        tree tmp = NULL_TREE;
        if (VECTOR_MODE_P (mode))
          return const_vector_from_tree (exp);
        scalar_int_mode int_mode;
        if (is_int_mode (mode, &int_mode))
          {
            if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
              return const_scalar_mask_from_tree (int_mode, exp);
where the VECTOR_CST type is now a vector boolean with DImode element type and
TImode as the (poor man's) vector mode.

So, the question is how to differentiate between vector booleans that want to
be a bitmask in an integral mode vs. poor man's vector booleans for which we'd
want to fallthru into the VIEW_CONVERT_EXPR code below this.
And what other spots need that.
Perhaps check if the bitsize (or precision?) of the vector type's mode is equal
to bitsize (or precision?) of the element mode times number of elements in the
vector?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (2 preceding siblings ...)
  2020-10-21 17:07 ` jakub at gcc dot gnu.org
@ 2020-10-21 18:33 ` marxin at gcc dot gnu.org
  2020-10-22  7:19 ` rguenth at gcc dot gnu.org
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-10-21 18:33 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
      Known to work|                            |10.2.0
           Priority|P3                          |P1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (3 preceding siblings ...)
  2020-10-21 18:33 ` marxin at gcc dot gnu.org
@ 2020-10-22  7:19 ` rguenth at gcc dot gnu.org
  2020-10-22  7:25 ` jakub at gcc dot gnu.org
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-22  7:19 UTC (permalink / raw)
  To: gcc-bugs

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

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
             Status|NEW                         |ASSIGNED

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> And we do that because:
>     case VECTOR_CST:
>       {
>         tree tmp = NULL_TREE;
>         if (VECTOR_MODE_P (mode))
>           return const_vector_from_tree (exp);
>         scalar_int_mode int_mode;
>         if (is_int_mode (mode, &int_mode))
>           {
>             if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
>               return const_scalar_mask_from_tree (int_mode, exp);

I think const_scalar_mask_from_tree is completely wrong in ignoring the
precision of the VECTOR_BOOLEAN component type.  At least it is
inconsistent with the processing of VECTOR_BOOLEAN_TYPE_P CTORs.

Now I can see that this was intended for AVX512 but I can't see how this
"inconsistency" can possibly work if you consider the use of _1:

  _19 = BIT_FIELD_REF <_1, 64, 0>;

if you disable TER then how should we ever able to "reinterpret" the
BIT_FIELD_REF to the "narrowed" view of the VECTOR_BOOLEAN_TYPE_P
mask?

A heuristic for an intermediate hack might be to skip
const_scalar_mask_from_tree when the component precision times the
number of elements equals the mode precision of the vector.

Thus the following fixes the testcase

diff --git a/gcc/expr.c b/gcc/expr.c
index 9d951e82522..d87bda777d0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -10356,7 +10356,10 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode
tmode,
        scalar_int_mode int_mode;
        if (is_int_mode (mode, &int_mode))
          {
-           if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
+           if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp))
+               && maybe_ne (TYPE_PRECISION (TREE_TYPE (TREE_TYPE (exp)))
+                            * TYPE_VECTOR_SUBPARTS (TREE_TYPE (exp)),
+                            GET_MODE_PRECISION (int_mode)))
              return const_scalar_mask_from_tree (int_mode, exp);
            else

but I have no easy access to a AVX512 runtime system (and no idea how to
make dejagnu use sde for a i386.exp testsuite run).  On a AVX2 system
i386.exp is clean with the above.

Thoughts?

[AVX512 should have used VnBImode like SVE does]

> where the VECTOR_CST type is now a vector boolean with DImode element type
> and TImode as the (poor man's) vector mode.
> 
> So, the question is how to differentiate between vector booleans that want
> to be a bitmask in an integral mode vs. poor man's vector booleans for which
> we'd want to fallthru into the VIEW_CONVERT_EXPR code below this.
> And what other spots need that.
> Perhaps check if the bitsize (or precision?) of the vector type's mode is
> equal to bitsize (or precision?) of the element mode times number of
> elements in the vector?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (4 preceding siblings ...)
  2020-10-22  7:19 ` rguenth at gcc dot gnu.org
@ 2020-10-22  7:25 ` jakub at gcc dot gnu.org
  2020-10-22  8:32 ` crazylht at gmail dot com
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-22  7:25 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I can test it on skylake-avx512.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (5 preceding siblings ...)
  2020-10-22  7:25 ` jakub at gcc dot gnu.org
@ 2020-10-22  8:32 ` crazylht at gmail dot com
  2020-10-22  8:45 ` rguenther at suse dot de
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: crazylht at gmail dot com @ 2020-10-22  8:32 UTC (permalink / raw)
  To: gcc-bugs

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

Hongtao.liu <crazylht at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |crazylht at gmail dot com

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Richard Biener from comment #4)
> (In reply to Jakub Jelinek from comment #3)
> > And we do that because:
> >     case VECTOR_CST:
> >       {
> >         tree tmp = NULL_TREE;
> >         if (VECTOR_MODE_P (mode))
> >           return const_vector_from_tree (exp);
> >         scalar_int_mode int_mode;
> >         if (is_int_mode (mode, &int_mode))
> >           {
> >             if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
> >               return const_scalar_mask_from_tree (int_mode, exp);
> 
> but I have no easy access to a AVX512 runtime system (and no idea how to
> make dejagnu use sde for a i386.exp testsuite run).  On a AVX2 system
> i386.exp is clean with the above.
> 
> Thoughts?
> 
> [AVX512 should have used VnBImode like SVE does]
> 


Can vec_merge support operands with VnBImode as items? I didn't find it's used
in any SVE patterns.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (6 preceding siblings ...)
  2020-10-22  8:32 ` crazylht at gmail dot com
@ 2020-10-22  8:45 ` rguenther at suse dot de
  2020-10-22  8:52 ` jakub at gcc dot gnu.org
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenther at suse dot de @ 2020-10-22  8:45 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 22 Oct 2020, crazylht at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97521
> 
> Hongtao.liu <crazylht at gmail dot com> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |crazylht at gmail dot com
> 
> --- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
> (In reply to Richard Biener from comment #4)
> > (In reply to Jakub Jelinek from comment #3)
> > > And we do that because:
> > >     case VECTOR_CST:
> > >       {
> > >         tree tmp = NULL_TREE;
> > >         if (VECTOR_MODE_P (mode))
> > >           return const_vector_from_tree (exp);
> > >         scalar_int_mode int_mode;
> > >         if (is_int_mode (mode, &int_mode))
> > >           {
> > >             if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
> > >               return const_scalar_mask_from_tree (int_mode, exp);
> > 
> > but I have no easy access to a AVX512 runtime system (and no idea how to
> > make dejagnu use sde for a i386.exp testsuite run).  On a AVX2 system
> > i386.exp is clean with the above.
> > 
> > Thoughts?
> > 
> > [AVX512 should have used VnBImode like SVE does]
> > 
> 
> 
> Can vec_merge support operands with VnBImode as items? I didn't find it's used
> in any SVE patterns.

You can use (vec_select (vec_concat ...)) instead of (vec_merge ...).
It's a long standing issue that we have two ways of expressing the same
(each with it's own drawbacks).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (7 preceding siblings ...)
  2020-10-22  8:45 ` rguenther at suse dot de
@ 2020-10-22  8:52 ` jakub at gcc dot gnu.org
  2020-10-22  8:58 ` jakub at gcc dot gnu.org
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-22  8:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, I think it is certainly desirable to keep using VEC_MERGE, not only
because it is less IL.
But pedantically already what i386 backend does doesn't match the documentation
which says that the last operand of VEC_MERGE is a CONST_INT.
We could just say that it is either a CONST_INT, or an RTL expression with a
scalar integral mode, or V*BImode mode.
And another question is how to represent the V*BImode constants, whether to
represent them as normal VOIDmode CONST_INTs, or something else
(const_vector:V*BI ...).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (8 preceding siblings ...)
  2020-10-22  8:52 ` jakub at gcc dot gnu.org
@ 2020-10-22  8:58 ` jakub at gcc dot gnu.org
  2020-10-22  9:00 ` crazylht at gmail dot com
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-22  8:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Looking at simplify-rtx.c, I think as long as the constants are CONST_INTs,
simplify-rtx.c doesn't care if the non-constant is represented with scalar int
mode registers (etc.) or V*BImode.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (9 preceding siblings ...)
  2020-10-22  8:58 ` jakub at gcc dot gnu.org
@ 2020-10-22  9:00 ` crazylht at gmail dot com
  2020-10-22  9:01 ` jakub at gcc dot gnu.org
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: crazylht at gmail dot com @ 2020-10-22  9:00 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Hongtao.liu <crazylht at gmail dot com> ---
Speaking about how to represent the V*BImode constants, i think we need to
extend attribute vector_size to handle something like
---
typedef bool v8bi __attribute__ ((vector_size (1)));
---

currently there would be an error as 

<source>:1:51: error: invalid vector type for attribute 'vector_size'

    1 | typedef bool v8qi __attribute__ ((vector_size (1)));


also we need to support type convert between vector boolean types and integer
in front end, since many users may use integer as a mask.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (10 preceding siblings ...)
  2020-10-22  9:00 ` crazylht at gmail dot com
@ 2020-10-22  9:01 ` jakub at gcc dot gnu.org
  2020-10-22  9:50 ` rguenther at suse dot de
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-10-22  9:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We don't necessarily need to make it user accessible.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (11 preceding siblings ...)
  2020-10-22  9:01 ` jakub at gcc dot gnu.org
@ 2020-10-22  9:50 ` rguenther at suse dot de
  2020-10-22 10:59 ` cvs-commit at gcc dot gnu.org
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenther at suse dot de @ 2020-10-22  9:50 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 22 Oct 2020, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97521
> 
> --- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> We don't necessarily need to make it user accessible.

Though I've missed a way to create VECTOR_BOOLEAN_TYPE_P vector
types for the GIMPLE FE (which measn the C FE has to have a way to
create them).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (12 preceding siblings ...)
  2020-10-22  9:50 ` rguenther at suse dot de
@ 2020-10-22 10:59 ` cvs-commit at gcc dot gnu.org
  2020-10-22 11:11 ` rguenth at gcc dot gnu.org
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-10-22 10:59 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 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:b960a9c83a93b58a84a7a370002990810675ac5d

commit r11-4226-gb960a9c83a93b58a84a7a370002990810675ac5d
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Oct 22 09:29:47 2020 +0200

    middle-end/97521 - fix VECTOR_CST expansion

    This fixes expansion of VECTOR_BOOLEAN_TYPE_P VECTOR_CSTs which
    when using an integer mode are not always "mask-mode" but may
    be using an integer mode when there's no supported vector mode.

    The patch makes sure to only go the mask-mode expansion if
    the elements do not line up to cover the full integer mode
    (when they do and the mode was an actual mask-mode there's
    no actual difference in both expansions).

    2020-10-22  Richard Biener  <rguenther@suse.de>

            PR middle-end/97521
            * expr.c (expand_expr_real_1): Be more careful when
            expanding a VECTOR_BOOLEAN_TYPE_P VECTOR_CSTs.

            * gcc.target/i386/pr97521.c: New testcase.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (13 preceding siblings ...)
  2020-10-22 10:59 ` cvs-commit at gcc dot gnu.org
@ 2020-10-22 11:11 ` rguenth at gcc dot gnu.org
  2020-10-23  6:04 ` rguenth at gcc dot gnu.org
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-22 11:11 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Accidentially pushed it before knowing AVX512 HW results.  Let's hope fixed.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (14 preceding siblings ...)
  2020-10-22 11:11 ` rguenth at gcc dot gnu.org
@ 2020-10-23  6:04 ` rguenth at gcc dot gnu.org
  2020-10-23  6:21 ` rguenth at gcc dot gnu.org
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-23  6:04 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
CI with -march=cascadelake reports

FAIL: c-c++-common/torture/vector-compare-2.c   -O0  execution test
FAIL: gcc.target/i386/avx2-vpcmpeqq-2.c execution test
FAIL: gfortran.dg/allocate_zerosize_3.f   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/allocate_zerosize_3.f   -O3 -g  execution test
FAIL: gfortran.dg/bound_6.f90   -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/bound_6.f90   -O3 -g  execution test

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (15 preceding siblings ...)
  2020-10-23  6:04 ` rguenth at gcc dot gnu.org
@ 2020-10-23  6:21 ` rguenth at gcc dot gnu.org
  2020-10-23  6:23 ` cvs-commit at gcc dot gnu.org
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-23  6:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #15)
> CI with -march=cascadelake reports
> 
..
> FAIL: gcc.target/i386/avx2-vpcmpeqq-2.c execution test

expands

(gdb) p debug_tree (exp)
 <vector_cst 0x7ffff4bbd390
    type <vector_type 0x7ffff683a3f0
        type <boolean_type 0x7ffff683a348 public QI
            size <integer_cst 0x7ffff680cdc8 constant 8>
            unit-size <integer_cst 0x7ffff680cde0 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff683a348 precision:2 min <integer_cst 0x7ffff66c9df8 -2> max <integer_cst
0x7ffff66ff288 1>>
        QI size <integer_cst 0x7ffff680cdc8 8> unit-size <integer_cst
0x7ffff680cde0 1>
        align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff683a3f0 nunits:4>
    constant tree_0 npatterns:2 nelts-per-pattern:2
    elt:0:  <integer_cst 0x7ffff4b9f738 type <boolean_type 0x7ffff683a348>
constant 0>
    elt:1:  <integer_cst 0x7ffff4b9f678 type <boolean_type 0x7ffff683a348>
constant -1> elt:2:  <integer_cst 0x7ffff4b9f738 0> elt:3:  <integer_cst
0x7ffff4b9f738 0>>

which shows the heuristic cannot work.  We possibly can refine it to
key on mode-precision component types - which _might_ work since it seems
x86 uses the smallest integer mode to hold nunits bits - but that's of course
not something guaranteed for non-x86.

I wonder why we're insisting to "fill" the mask mode on GENERIC/GIMPLE
while RTL produces packed bits.  Thus, why do we use a
QImode vector(4) <signed-boolean:2> here instead of a
QImode vector(4) <signed-boolean:1> if the target in the end will produce that
from say, a V4SImode compare-to-mask?  As long as we didn't expose
temporaries of those types this was well-hidden up to RTL expansion which
then did "magic" but now we're really facing inconsistent representations.

Now targets _could_ opt to use QImode vector(4) <signed-boolean:2> but then
with representing { -1, -1, -1, -1 } as 0b11111111 (with the 'padding bits'
sign-extended).

For now I'm going to revert the patch but I still believe
const_scalar_mask_from_tree is a red herring.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (16 preceding siblings ...)
  2020-10-23  6:21 ` rguenth at gcc dot gnu.org
@ 2020-10-23  6:23 ` cvs-commit at gcc dot gnu.org
  2020-10-23  6:58 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-10-23  6:23 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 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:7cda498920dbf244e9e06fdb2fc710a118a8c033

commit r11-4278-g7cda498920dbf244e9e06fdb2fc710a118a8c033
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Oct 23 08:21:39 2020 +0200

    Revert "middle-end/97521 - fix VECTOR_CST expansion"

    2020-10-23  Richard Biener  <rguenther@suse.de>

            PR middle-end/97521
            * expr.c (expand_expr_real_1): Revert last change.

            * gcc.target/i386/pr97521.c: Remove.
    This reverts commit b960a9c83a93b58a84a7a370002990810675ac5d.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (17 preceding siblings ...)
  2020-10-23  6:23 ` cvs-commit at gcc dot gnu.org
@ 2020-10-23  6:58 ` rguenth at gcc dot gnu.org
  2020-10-23  7:00 ` rguenth at gcc dot gnu.org
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-23  6:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
Maybe the x86 backend should use partial integer modes here (though we'd have
quite some, eventually not even possible), so the mask mode precision would
tell us how to build the types.  Or the targetm.vectorize.get_mask_mode
needs to be amended to return the number of bits used per lane.

So the following "works" for the testcase in this bug and the FAILs reported
but of course breaks the non-AVX512 case because we cannot distinguish
between integer mode vectors and integer mode masks :/  The target hook
docs leave the option to return no mask mode but the default implementation
would already always return an integer vector mode if it exists.

So given that changing x86 to use MODE_VECTOR_BOOL is unlikely we need a way
to distinguish MODE_INT vectors from MODE_INT vector bools, thus, the target
needs to tell us the precision of the elements.

diff --git a/gcc/expr.c b/gcc/expr.c
index 9d951e82522..ae16f077758 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -10356,16 +10355,10 @@ expand_expr_real_1 (tree exp, rtx target,
machine_mode tmode,
        scalar_int_mode int_mode;
        if (is_int_mode (mode, &int_mode))
          {
-           if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
-             return const_scalar_mask_from_tree (int_mode, exp);
-           else
-             {
-               tree type_for_mode
-                 = lang_hooks.types.type_for_mode (int_mode, 1);
-               if (type_for_mode)
-                 tmp = fold_unary_loc (loc, VIEW_CONVERT_EXPR,
-                                       type_for_mode, exp);
-             }
+           tree type_for_mode = lang_hooks.types.type_for_mode (int_mode, 1);
+           if (type_for_mode)
+             tmp = fold_unary_loc (loc, VIEW_CONVERT_EXPR,
+                                   type_for_mode, exp);
          }
        if (!tmp)
          {
diff --git a/gcc/tree.c b/gcc/tree.c
index 555ba97e68b..6eeee08f124 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10926,9 +10926,7 @@ build_truth_vector_type_for_mode (poly_uint64 nunits,
machine_mode mask_mode)
 {
   gcc_assert (mask_mode != BLKmode);

-  poly_uint64 vsize = GET_MODE_BITSIZE (mask_mode);
-  unsigned HOST_WIDE_INT esize = vector_element_size (vsize, nunits);
-  tree bool_type = build_nonstandard_boolean_type (esize);
+  tree bool_type = build_nonstandard_boolean_type (1);

   return make_vector_type (bool_type, nunits, mask_mode);
 }

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (18 preceding siblings ...)
  2020-10-23  6:58 ` rguenth at gcc dot gnu.org
@ 2020-10-23  7:00 ` rguenth at gcc dot gnu.org
  2020-10-23  7:06 ` rguenth at gcc dot gnu.org
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-23  7:00 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCN also uses MODE_INT for the mask mode and thus may be similarly affected.
Andrew - are the bits in the mask dense?  Thus for a V4SImode compare
would the mask occupy only the lowest 4 bits of the DImode mask?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (19 preceding siblings ...)
  2020-10-23  7:00 ` rguenth at gcc dot gnu.org
@ 2020-10-23  7:06 ` rguenth at gcc dot gnu.org
  2020-10-23  8:28 ` ams at gcc dot gnu.org
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-23  7:06 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
So at least the tree.c use of get_mask_mode only passes it MODE_VECTOR so
we don't have to second-guess the component size when passed a MODE_INT used
as representation for an integer vector type.

So I'm testing the following

tree
build_truth_vector_type_for_mode (poly_uint64 nunits, machine_mode mask_mode)
{
  gcc_assert (mask_mode != BLKmode);

  unsigned HOST_WIDE_INT esize;
  if (VECTOR_MODE_P (mask_mode))
    {
      poly_uint64 vsize = GET_MODE_BITSIZE (mask_mode);
      esize = vector_element_size (vsize, nunits);
    }
  else
    esize = 1;

  tree bool_type = build_nonstandard_boolean_type (esize);

  return make_vector_type (bool_type, nunits, mask_mode);
}

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (20 preceding siblings ...)
  2020-10-23  7:06 ` rguenth at gcc dot gnu.org
@ 2020-10-23  8:28 ` ams at gcc dot gnu.org
  2020-10-23  8:47 ` ams at gcc dot gnu.org
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 26+ messages in thread
From: ams at gcc dot gnu.org @ 2020-10-23  8:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #21 from Andrew Stubbs <ams at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #19)
> GCN also uses MODE_INT for the mask mode and thus may be similarly affected.
> Andrew - are the bits in the mask dense?  Thus for a V4SImode compare
> would the mask occupy only the lowest 4 bits of the DImode mask?

Yes, that's correct.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (21 preceding siblings ...)
  2020-10-23  8:28 ` ams at gcc dot gnu.org
@ 2020-10-23  8:47 ` ams at gcc dot gnu.org
  2020-10-26 11:28 ` cvs-commit at gcc dot gnu.org
  2020-10-26 11:28 ` rguenth at gcc dot gnu.org
  24 siblings, 0 replies; 26+ messages in thread
From: ams at gcc dot gnu.org @ 2020-10-23  8:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #22 from Andrew Stubbs <ams at gcc dot gnu.org> ---
(In reply to Andrew Stubbs from comment #21)
> (In reply to Richard Biener from comment #19)
> > GCN also uses MODE_INT for the mask mode and thus may be similarly affected.
> > Andrew - are the bits in the mask dense?  Thus for a V4SImode compare
> > would the mask occupy only the lowest 4 bits of the DImode mask?
> 
> Yes, that's correct.

Or rather, I should say that that *will* be the case when I add partial vector
support; right now it can only be done via masking V64SImode.

A have a patch set, but the last problem is that while_ult doesn't operate on
partial integer masks, leading to wrong code. AArch64 doesn't have a problem
with this because it uses VBI masks of the right size. I have a patch that adds
the vector size as an operand to while_ult; this seems to fix the problems on
GCN, but I need to make corresponding changes for AArch64 also before I can
submit those patches, and time is tight.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (22 preceding siblings ...)
  2020-10-23  8:47 ` ams at gcc dot gnu.org
@ 2020-10-26 11:28 ` cvs-commit at gcc dot gnu.org
  2020-10-26 11:28 ` rguenth at gcc dot gnu.org
  24 siblings, 0 replies; 26+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-10-26 11:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #23 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:605c2a393d3a2db86454a70fd7c9467db434060c

commit r11-4381-g605c2a393d3a2db86454a70fd7c9467db434060c
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Oct 23 08:40:15 2020 +0200

    middle-end/97521 - always use single-bit bools in mask vector types

    This makes us always use a single-bit boolean type component type
    for integer mode mask VECTOR_BOOLEAN_TYPE_P to match the RTL and target
    representation.  This aovids the need for magic translation and
    the inconsistencies from the translation requirement now that
    we expose temporaries of those types on the GIMPLE level.

    2020-10-23  Richard Biener  <rguenther@suse.de>

            PR middle-end/97521
            * expr.c (const_scalar_mask_from_tree): Remove.
            (expand_expr_real_1): Always VIEW_CONVERT integer mode
            vector constants to an integer type.
            * tree.c (build_truth_vector_type_for_mode): Use a single-bit
            boolean component type for non-vector-mode mask_mode.

            * gcc.target/i386/pr97521.c: New testcase.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394
  2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
                   ` (23 preceding siblings ...)
  2020-10-26 11:28 ` cvs-commit at gcc dot gnu.org
@ 2020-10-26 11:28 ` rguenth at gcc dot gnu.org
  24 siblings, 0 replies; 26+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-10-26 11:28 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-10-26 11:28 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 16:27 [Bug target/97521] New: [11 Regression] wrong code with -mno-sse2 zsojka at seznam dot cz
2020-10-21 16:37 ` [Bug target/97521] [11 Regression] wrong code with -mno-sse2 since r11-3394 jakub at gcc dot gnu.org
2020-10-21 16:56 ` jakub at gcc dot gnu.org
2020-10-21 17:07 ` jakub at gcc dot gnu.org
2020-10-21 18:33 ` marxin at gcc dot gnu.org
2020-10-22  7:19 ` rguenth at gcc dot gnu.org
2020-10-22  7:25 ` jakub at gcc dot gnu.org
2020-10-22  8:32 ` crazylht at gmail dot com
2020-10-22  8:45 ` rguenther at suse dot de
2020-10-22  8:52 ` jakub at gcc dot gnu.org
2020-10-22  8:58 ` jakub at gcc dot gnu.org
2020-10-22  9:00 ` crazylht at gmail dot com
2020-10-22  9:01 ` jakub at gcc dot gnu.org
2020-10-22  9:50 ` rguenther at suse dot de
2020-10-22 10:59 ` cvs-commit at gcc dot gnu.org
2020-10-22 11:11 ` rguenth at gcc dot gnu.org
2020-10-23  6:04 ` rguenth at gcc dot gnu.org
2020-10-23  6:21 ` rguenth at gcc dot gnu.org
2020-10-23  6:23 ` cvs-commit at gcc dot gnu.org
2020-10-23  6:58 ` rguenth at gcc dot gnu.org
2020-10-23  7:00 ` rguenth at gcc dot gnu.org
2020-10-23  7:06 ` rguenth at gcc dot gnu.org
2020-10-23  8:28 ` ams at gcc dot gnu.org
2020-10-23  8:47 ` ams at gcc dot gnu.org
2020-10-26 11:28 ` cvs-commit at gcc dot gnu.org
2020-10-26 11:28 ` rguenth at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).