public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
@ 2020-12-09  8:52 marxin at gcc dot gnu.org
  2020-12-09  8:52 ` [Bug tree-optimization/98211] " marxin at gcc dot gnu.org
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-12-09  8:52 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 98211
           Summary: [11 Regression] Wrong code at -O3 since
                    r11-4482-gb626b00823af9ca9
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org
  Target Milestone: ---

Reduced from a yarpgen test-case. Started with r11-4482-gb626b00823af9ca9 when
a new SLP opportunity happens.

$ cat pr.C
unsigned long long seed;
void hash(unsigned long long *seed, int v) { *seed ^= v; }

long var_11 = -4439057030159136246;
short arr_19[8][2][16];
void test(unsigned long long, bool, unsigned long long);

void
__attribute__((noipa, optimize("-O0")))
calculate_hash(void)
{
  for (long i_0 = 0; i_0 < 8; ++i_0)
    for (long i_1 = 0; i_1 < 2; ++i_1)
      for (long i_2 = 0; i_2 < 16; ++i_2)
        hash(&seed, arr_19[i_0][i_1][i_2]);
}

int main() {
  test(407382096781995, 0, 4);
  calculate_hash ();

  __builtin_printf("%llu\n", seed);
  if (seed != 2570)
    __builtin_abort ();

  return 0;
}

long val_or_zero(long __b) {
  if (__b)
    return __b;
  return 0;
}

int test_var_3;
int counter = 0;

void test(unsigned long long var_1, bool var_9, unsigned long long var_12) {
  for (unsigned i_0 = 0; i_0 < 7; i_0 += 9)
    for (bool i_1 = 0; i_1 < (bool)var_1; i_1 += 4)
      for (unsigned i_5 = 0; i_5 < (int)val_or_zero(var_11) - 1940523515U;
i_5++)
        arr_19[i_0][i_1][i_5] = (test_var_3 ?: var_12) ? (short)var_11 : var_9;
}

$ gcc -O3 pr.C && ./a.out
0
Aborted (core dumped)

$ gcc pr.C && ./a.out
2570

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
@ 2020-12-09  8:52 ` marxin at gcc dot gnu.org
  2020-12-09  9:09 ` rguenth at gcc dot gnu.org
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-12-09  8:52 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
      Known to work|                            |10.2.0
      Known to fail|                            |11.0
   Target Milestone|---                         |11.0
   Last reconfirmed|                            |2020-12-09
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P1

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
  2020-12-09  8:52 ` [Bug tree-optimization/98211] " marxin at gcc dot gnu.org
@ 2020-12-09  9:09 ` rguenth at gcc dot gnu.org
  2020-12-09 10:39 ` rguenth at gcc dot gnu.org
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09  9:09 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
  2020-12-09  8:52 ` [Bug tree-optimization/98211] " marxin at gcc dot gnu.org
  2020-12-09  9:09 ` rguenth at gcc dot gnu.org
@ 2020-12-09 10:39 ` rguenth at gcc dot gnu.org
  2020-12-09 10:44 ` rguenth at gcc dot gnu.org
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09 10:39 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Huh.  OK, so we do some pointless vectorization (the store is in a BB
ending in __builtin_unreachable()) but the actual issue must be the
live lane extraction into the not vectorized scalar code:

  vect_patt_95.26_71 = .VCOND_MASK (mask_patt_91.25_47, _59, _67);
  _79 = BIT_FIELD_REF <vect_patt_95.26_71, 16, 0>;

hmm, somehow the VCOND_MASK condition unpacking ends up with
v16_int8 = {1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0}

Looks like this is because we have

  _17 = var_12_18(D) != 0;
  _11 = test_var_3.4_1 != 0;
  _26 = _11 | _17;
  _99 = VIEW_CONVERT_EXPR<unsigned char>(_26);
  _35 = {_99, _99, _99, _99, _99, _99, _99, _99, _99, _99, _99, _99, _99, _99,
_99, _99};
  mask_patt_87.23_39 = VIEW_CONVERT_EXPR<vector(16) <signed-boolean:8>>(_35);
  mask_patt_91.25_47 = [vec_unpack_lo_expr] mask_patt_87.23_39;

but that doesn't produce the canonical -1 values which means the bool
pattern is somehow broke.  We do code-generate

  mask_patt_87.23_39 = VIEW_CONVERT_EXPR<vector(16) <signed-boolean:8>>(_35);
  mask_patt_84.24_43 = mask_patt_87.23_39 ^ { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0 };
  mask_patt_91.25_47 = [vec_unpack_lo_expr] mask_patt_84.24_43;

from the

(gdb) p debug (slp_node)
x.c:37:6: note: node 0x3f7e078 (max_nunits=16, refcnt=1)
x.c:37:6: note: op template: patt_84 = patt_87 != 0;
x.c:37:6: note:         stmt 0 patt_84 = patt_87 != 0;
x.c:37:6: note:         stmt 1 patt_84 = patt_87 != 0;
...

node by choosing BIT_XOR.  patt_87 has boolean vector type, but that
is just a cast:

x.c:41:31: note:   op template: patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:        stmt 0 patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:        stmt 1 patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:        stmt 2 patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:        stmt 3 patt_87 = (<signed-boolean:8>) _26;

which I guess is what is wrong, built via

#0  0x0000000002a217c0 in build_mask_conversion (vinfo=0x3e3e510,
mask=<ssa_name 0x7ffff69a3d80 26>, 
    vectype=<vector_type 0x7ffff699ad20>, stmt_vinfo=0x3e64490)
    at /home/rguenther/src/gcc2/gcc/tree-vect-patterns.c:4230
#1  0x0000000002a22497 in vect_recog_mask_conversion_pattern (vinfo=0x3e3e510,
stmt_vinfo=0x3e64490, 
    type_out=0x7fffffffd060) at
/home/rguenther/src/gcc2/gcc/tree-vect-patterns.c:4457
#2  0x0000000002a25505 in vect_pattern_recog_1 (vinfo=0x3e3e510, 
    recog_func=0x3b21050 <vect_vect_recog_func_ptrs+272>, stmt_info=0x3e64490)
    at /home/rguenther/src/gcc2/gcc/tree-vect-patterns.c:5450
#3  0x0000000002a25a1f in vect_pattern_recog (vinfo=0x3e3e510)
    at /home/rguenther/src/gcc2/gcc/tree-vect-patterns.c:5608

which is

      /* If rhs1 is a comparison we need to move it into a
         separate statement.  */
      if (TREE_CODE (rhs1) != SSA_NAME)
        {
          tmp = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL);
          if (rhs1_op0_type
              && TYPE_PRECISION (rhs1_op0_type) != TYPE_PRECISION (rhs1_type))
            rhs1_op0 = build_mask_conversion (vinfo, rhs1_op0,
                                              vectype2, stmt_vinfo);
          if (rhs1_op1_type
              && TYPE_PRECISION (rhs1_op1_type) != TYPE_PRECISION (rhs1_type))
            rhs1_op1 = build_mask_conversion (vinfo, rhs1_op1,
                                      vectype2, stmt_vinfo);
          pattern_stmt = gimple_build_assign (tmp, TREE_CODE (rhs1),
                                              rhs1_op0, rhs1_op1);
          rhs1 = tmp;
          append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, vectype2,
                                  rhs1_type);
        }

x.c:41:31: note:   === vect_determine_precisions ===
x.c:41:31: note:   using normal nonmask vectors for _17 = var_12_18(D) != 0;
x.c:41:31: note:   using boolean precision 32 for _11 = test_var_3.4_1 != 0;
x.c:41:31: note:   using boolean precision 32 for _26 = _11 | _17;
...
x.c:41:31: note:   === vect_pattern_recog ===
x.c:41:31: note:   vect_recog_mask_conversion_pattern: detected: iftmp.2_10 =
_26 != 0 ? iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   mask_conversion pattern recognized: patt_95 = patt_91 ?
iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   extra pattern stmt: patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:   extra pattern stmt: patt_84 = patt_87 != 0;
x.c:41:31: note:   extra pattern stmt: patt_91 = (<signed-boolean:16>) patt_84;

note _26 is not part of the SLP but is splat from the scalar def.

As SLP improvement it's to say we should have splat iftmp.2_10 itself but
the change probably disabled that.

Still the above is a latent issue - I'll try to craft a more meaningful
testcase.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2020-12-09 10:39 ` rguenth at gcc dot gnu.org
@ 2020-12-09 10:44 ` rguenth at gcc dot gnu.org
  2020-12-09 11:26 ` rguenth at gcc dot gnu.org
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09 10:44 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, maybe the pattern recog mask precision compute assumes all statements will
necessarily participate in the vectorization ...?

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2020-12-09 10:44 ` rguenth at gcc dot gnu.org
@ 2020-12-09 11:26 ` rguenth at gcc dot gnu.org
  2020-12-09 11:47 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09 11:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GIMPLE testcase for the vectorization (w/o driver part).  If you make the
condition non-uniform then we include the _28 SLP def in the SLP and
everything goes correct.  So it might be that we need to special-case
"external bools", but I thought we pattern those as != 0 - guess we elide
that when we see the BIT_IOR_EXPR use (but that would assume we vectorize
that part as well).

int test_var_3;
short arr_20[16];
int testa[1024];

void __GIMPLE (ssa,startwith("slp"))
test (long long unsigned int var_1, long long unsigned int var_12,
      short int a, short int b, short int c, short int d)
{
  _Bool tem2;
  _Bool tem;
  unsigned int i_5;
  int _24;
  _Bool _28;
  short int _30;
  short int _32;

  __BB(2):
  _24 = test_var_3;
  tem_25 = _24 != 0;
  tem2_26 = var_1_11(D) != 0ul;
  _28 = tem_25 | tem2_26;
  _30 = _28 !=  _Literal (_Bool) 0 ? a_16(D) : b_15(D);
  arr_20[0u] = _30;
  _32 = _28 != _Literal (_Bool) 0 ? c_19(D) : d_18(D);
  arr_20[8u] = _32;
  arr_20[1u] = _30;
  arr_20[9u] = _32;
  arr_20[2u] = _30;
  arr_20[10u] = _32;
  arr_20[3u] = _30;
  arr_20[11u] = _32;
  arr_20[4u] = _30;
  arr_20[12u] = _32;
  arr_20[5u] = _30;
  arr_20[13u] = _32;
  arr_20[6u] = _30;
  arr_20[14u] = _32;
  arr_20[7u] = _30;
  arr_20[15u] = _32;
  return;

}

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2020-12-09 11:26 ` rguenth at gcc dot gnu.org
@ 2020-12-09 11:47 ` rguenth at gcc dot gnu.org
  2020-12-09 12:43 ` rguenth at gcc dot gnu.org
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09 11:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, so I think

x.c:41:31: note:   === vect_pattern_recog ===
x.c:41:31: note:   vect_recog_mask_conversion_pattern: detected: iftmp.2_10 =
_26 != 0 ? iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   mask_conversion pattern recognized: patt_95 = patt_91 ?
iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   extra pattern stmt: patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:   extra pattern stmt: patt_84 = patt_87 != 0;
x.c:41:31: note:   extra pattern stmt: patt_91 = (<signed-boolean:16>) patt_84;

needs to be recognized as bool pattern and thus this is a pattern ordering
issue and vect_recog_bool_pattern has to come after
vect_recog_mask_conversion_pattern.  Now I'm sure there's a thing the other way
around :/

diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index f2ce75aac3e..6a5f9505792 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -5267,12 +5267,12 @@ static vect_recog_func vect_vect_recog_func_ptrs[] = {
   { vect_recog_divmod_pattern, "divmod" },
   { vect_recog_mult_pattern, "mult" },
   { vect_recog_mixed_size_cond_pattern, "mixed_size_cond" },
-  { vect_recog_bool_pattern, "bool" },
   /* This must come before mask conversion, and includes the parts
      of mask conversion that are needed for gather and scatter
      internal functions.  */
   { vect_recog_gather_scatter_pattern, "gather_scatter" },
   { vect_recog_mask_conversion_pattern, "mask_conversion" },
+  { vect_recog_bool_pattern, "bool" },
   { vect_recog_widen_plus_pattern, "widen_plus" },
   { vect_recog_widen_minus_pattern, "widen_minus" },
 };

fixes the testcase and properly recognizes

x.c:41:31: note:   vect_recog_mask_conversion_pattern: detected: iftmp.2_10 =
_26 != 0 ? iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   mask_conversion pattern recognized: patt_95 = patt_91 ?
iftmp.2_22 : iftmp.2_21;
x.c:41:31: note:   extra pattern stmt: patt_87 = (<signed-boolean:8>) _26;
x.c:41:31: note:   extra pattern stmt: patt_84 = patt_87 != 0;
x.c:41:31: note:   extra pattern stmt: patt_91 = (<signed-boolean:16>) patt_84;
x.c:41:31: note:   vect_recog_bool_pattern: detected: patt_87 =
(<signed-boolean:8>) _26;
x.c:41:31: note:   bool pattern recognized: patt_103 = (<signed-boolean:8>)
patt_99;
x.c:41:31: note:   replacing earlier pattern patt_87 = (<signed-boolean:8>)
_26;
x.c:41:31: note:   with patt_87 = (<signed-boolean:8>) patt_99;
x.c:41:31: note:   extra pattern stmt: patt_99 = _26 ? 1 : 0;
x.c:41:31: note:   vect_recog_bool_pattern: detected: patt_91 =
(<signed-boolean:16>) patt_84;
x.c:41:31: note:   bool pattern recognized: patt_111 = (<signed-boolean:16>)
patt_107;
x.c:41:31: note:   replacing earlier pattern patt_91 = (<signed-boolean:16>)
patt_84;
x.c:41:31: note:   with patt_91 = (<signed-boolean:16>) patt_107;
x.c:41:31: note:   extra pattern stmt: patt_107 = patt_84 ? 1 : 0;

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2020-12-09 11:47 ` rguenth at gcc dot gnu.org
@ 2020-12-09 12:43 ` rguenth at gcc dot gnu.org
  2020-12-10 10:01 ` rguenth at gcc dot gnu.org
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-09 12:43 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |duanbo3 at huawei dot com,
                   |                            |rsandifo at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, ICEs all over the place as expected.

Btw, on the GCC 10 branch we had

diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 6a5f9505792..52725f826c7 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -4452,16 +4452,7 @@ vect_recog_mask_conversion_pattern (vec_info *vinfo,
       if (TREE_CODE (rhs1) != SSA_NAME)
        {
          tmp = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL);
-         if (rhs1_op0_type
-             && TYPE_PRECISION (rhs1_op0_type) != TYPE_PRECISION (rhs1_type))
-           rhs1_op0 = build_mask_conversion (vinfo, rhs1_op0,
-                                             vectype2, stmt_vinfo);
-         if (rhs1_op1_type
-             && TYPE_PRECISION (rhs1_op1_type) != TYPE_PRECISION (rhs1_type))
-           rhs1_op1 = build_mask_conversion (vinfo, rhs1_op1,
-                                     vectype2, stmt_vinfo);
-         pattern_stmt = gimple_build_assign (tmp, TREE_CODE (rhs1),
-                                             rhs1_op0, rhs1_op1);
+         pattern_stmt = gimple_build_assign (tmp, rhs1);
          rhs1 = tmp;
          append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, vectype2,
                                  rhs1_type);

and going back fixes the issue as well.  That was changed by g:91d80cf4bd28,
approved by Richard.

That said, using build_mask_conversion on sth not already a mask is definitely
wrong as can be seen here.  The above conversions should, according to the
revs comment, only happen when rhs1_op0_type != rhs1_op1_type (those are
scalar types here - but revs comment says vector types mismatch).

That said, if we throw bool patterns ontop of the mask conversion one we end
up not vectorizing the thing because

x3.c:23:9: note:   ==> examining statement: patt_22 = _28 ? 1 : 0;
x3.c:6:1: missed:   not vectorized: relevant stmt not supported: patt_22 = _28
? 1 : 0;

which is because vect_is_simple_cond runs into

  /* Mask case.  */        
  if (TREE_CODE (cond) == SSA_NAME
      && VECT_SCALAR_BOOLEAN_TYPE_P (TREE_TYPE (cond)))
    {
      if (!vect_is_simple_use (vinfo, stmt_info, slp_node, 0, &cond,
                               &slp_op, &dts[0], comp_vectype)
          || !*comp_vectype
          || !VECTOR_BOOLEAN_TYPE_P (*comp_vectype))
        return false;

so I guess that should end up as mask conversion pattern as well, recursively.

Meh.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2020-12-09 12:43 ` rguenth at gcc dot gnu.org
@ 2020-12-10 10:01 ` rguenth at gcc dot gnu.org
  2020-12-10 10:28 ` rsandifo at gcc dot gnu.org
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-10 10:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, OK, so besides the incomplete bool pattern matching the issue seems to be
that while we reject the problematic conversion in vectorizable_conversion it
slips through via vectorizable_assignment because it does

      /* Conversion between boolean types of different sizes is
         a simple assignment in case their vectypes are same
         boolean vectors.  */
      && (!VECTOR_BOOLEAN_TYPE_P (vectype)
          || !VECTOR_BOOLEAN_TYPE_P (vectype_in)))

as opposed to vectorizable_conversions

  if (VECTOR_BOOLEAN_TYPE_P (vectype_out)
      && !VECTOR_BOOLEAN_TYPE_P (vectype_in))

that was added by g:2dab46d5fc9f95de16bd9bf0f219be5e64324d1f without a testcase
or PR reference so it's difficult to tell what it was supposed to allow.  Now,
for the case in question the conversion would have slipped though anyway since
the only difference in the types is the sign and that one is BOOLEAN_TYPE
and the other INTEGER_TYPE.  So the exception above seems to intent to allow
conversions with different precisions (note we now have precision 1 for all
vector bools).

So I'm going to just copy the vectorizable_conversion condition into
vectorizable_assignment as well.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2020-12-10 10:01 ` rguenth at gcc dot gnu.org
@ 2020-12-10 10:28 ` rsandifo at gcc dot gnu.org
  2020-12-10 11:36 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2020-12-10 10:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Hmm, OK, so besides the incomplete bool pattern matching the issue seems to
> be that while we reject the problematic conversion in
> vectorizable_conversion it slips through via vectorizable_assignment because
> it does
> 
>       /* Conversion between boolean types of different sizes is
>          a simple assignment in case their vectypes are same
>          boolean vectors.  */
>       && (!VECTOR_BOOLEAN_TYPE_P (vectype)
>           || !VECTOR_BOOLEAN_TYPE_P (vectype_in)))
> 
> as opposed to vectorizable_conversions
> 
>   if (VECTOR_BOOLEAN_TYPE_P (vectype_out)
>       && !VECTOR_BOOLEAN_TYPE_P (vectype_in))
> 
> that was added by g:2dab46d5fc9f95de16bd9bf0f219be5e64324d1f without a
> testcase
> or PR reference so it's difficult to tell what it was supposed to allow. 
Agree that looks odd…

> Now,
> for the case in question the conversion would have slipped though anyway
> since
> the only difference in the types is the sign and that one is BOOLEAN_TYPE
> and the other INTEGER_TYPE.  So the exception above seems to intent to allow
> conversions with different precisions (note we now have precision 1 for all
> vector bools).
> 
> So I'm going to just copy the vectorizable_conversion condition into
> vectorizable_assignment as well.
Shouldn't it instead be:

  VECTOR_BOOLEAN_TYPE_P (vectype) != VECTOR_BOOLEAN_TYPE_P (vectype_in)

as is used in some other places (sometimes with ^ instead of !=)?
AFAIK using VCE would break in both directions.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2020-12-10 10:28 ` rsandifo at gcc dot gnu.org
@ 2020-12-10 11:36 ` cvs-commit at gcc dot gnu.org
  2020-12-10 11:37 ` rguenth at gcc dot gnu.org
  2020-12-10 11:41 ` rguenth at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-10 11:36 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 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:76c09f2af9d8ab9c760d860626f069d12d86f0a9

commit r11-5901-g76c09f2af9d8ab9c760d860626f069d12d86f0a9
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Dec 10 11:12:53 2020 +0100

    tree-optimization/98211 - fix bogus vectorization of conversion

    Pattern recog incompletely handles some bool cases but we shouldn't
    miscompile as a result but not vectorize.  Unfortunately
    vectorizable_assignment lets invalid conversions (that
    vectorizable_conversion rejects) slip through.  The following
    rectifies that.

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

            PR tree-optimization/98211
            * tree-vect-stmts.c (vectorizable_assignment): Disallow
            invalid conversions to bool vector types.

            * gcc.dg/pr98211.c: New testcase.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2020-12-10 11:36 ` cvs-commit at gcc dot gnu.org
@ 2020-12-10 11:37 ` rguenth at gcc dot gnu.org
  2020-12-10 11:41 ` rguenth at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-10 11:37 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed, though the fixed issue is latent.

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

* [Bug tree-optimization/98211] [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9
  2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2020-12-10 11:37 ` rguenth at gcc dot gnu.org
@ 2020-12-10 11:41 ` rguenth at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-10 11:41 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #8)
> (In reply to Richard Biener from comment #7)
> > Hmm, OK, so besides the incomplete bool pattern matching the issue seems to
> > be that while we reject the problematic conversion in
> > vectorizable_conversion it slips through via vectorizable_assignment because
> > it does
> > 
> >       /* Conversion between boolean types of different sizes is
> >          a simple assignment in case their vectypes are same
> >          boolean vectors.  */
> >       && (!VECTOR_BOOLEAN_TYPE_P (vectype)
> >           || !VECTOR_BOOLEAN_TYPE_P (vectype_in)))
> > 
> > as opposed to vectorizable_conversions
> > 
> >   if (VECTOR_BOOLEAN_TYPE_P (vectype_out)
> >       && !VECTOR_BOOLEAN_TYPE_P (vectype_in))
> > 
> > that was added by g:2dab46d5fc9f95de16bd9bf0f219be5e64324d1f without a
> > testcase
> > or PR reference so it's difficult to tell what it was supposed to allow. 
> Agree that looks odd…
> 
> > Now,
> > for the case in question the conversion would have slipped though anyway
> > since
> > the only difference in the types is the sign and that one is BOOLEAN_TYPE
> > and the other INTEGER_TYPE.  So the exception above seems to intent to allow
> > conversions with different precisions (note we now have precision 1 for all
> > vector bools).
> > 
> > So I'm going to just copy the vectorizable_conversion condition into
> > vectorizable_assignment as well.
> Shouldn't it instead be:
> 
>   VECTOR_BOOLEAN_TYPE_P (vectype) != VECTOR_BOOLEAN_TYPE_P (vectype_in)
> 
> as is used in some other places (sometimes with ^ instead of !=)?
> AFAIK using VCE would break in both directions.

I think the intent was to allow vector<bool:2> to vector<bool:4>, both SImode
to be converted for free (since in the end they have the same representation,
_not_ occupying 2 and 4 bits but only one).  I've recently "fixed" (changed)
the integer mode bool vectors to always be vector<bool:1> to reflect that
so I'm going to test simply removing that odd condition.

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

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

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09  8:52 [Bug tree-optimization/98211] New: [11 Regression] Wrong code at -O3 since r11-4482-gb626b00823af9ca9 marxin at gcc dot gnu.org
2020-12-09  8:52 ` [Bug tree-optimization/98211] " marxin at gcc dot gnu.org
2020-12-09  9:09 ` rguenth at gcc dot gnu.org
2020-12-09 10:39 ` rguenth at gcc dot gnu.org
2020-12-09 10:44 ` rguenth at gcc dot gnu.org
2020-12-09 11:26 ` rguenth at gcc dot gnu.org
2020-12-09 11:47 ` rguenth at gcc dot gnu.org
2020-12-09 12:43 ` rguenth at gcc dot gnu.org
2020-12-10 10:01 ` rguenth at gcc dot gnu.org
2020-12-10 10:28 ` rsandifo at gcc dot gnu.org
2020-12-10 11:36 ` cvs-commit at gcc dot gnu.org
2020-12-10 11:37 ` rguenth at gcc dot gnu.org
2020-12-10 11:41 ` 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).