On Mon, 15 Feb 2016, Jakub Jelinek wrote: > Hi! > > The following testcase is miscompiled, because we first > create a pattern stmt for _5 = (int) _3; where _3 is bool, > but then recognize the following multiply as widening multiply, ignore > there the previous pattern stmt and thus instead of expanding the > cast as cond ? 1 : 0 we actually end up expanding it as cond (i.e. > cond ? -1 : 0). In the widen mult pattern recognizer it perhaps would be > possible to handle that case, unless both arguments are cast from > bool, but there are lots of other places which call type_conversion_p and > most of them would need to either give up in those cases or add special > handling for it. So, it seems like the easiest fix at least for GCC6 is > to punt in type_conversion_p on casts from bool/unsigned :1 (or should > I instead check STMT_VINFO_IN_PATTERN_P on the cast stmt and give up > if true?). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Hmm, most places calling type_conversion_p already check for the pattern case and only accept a very specific or fail. Only widen_mul/sum don't do that. I believe type_conversion_p should simply look at the detected pattern, thus Index: tree-vect-patterns.c =================================================================== --- tree-vect-patterns.c (revision 233418) +++ tree-vect-patterns.c (working copy) @@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us if (!*def_stmt) return false; + if (dt == vect_internal_def) + { + stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt); + if (STMT_VINFO_IN_PATTERN_P (def_vinfo)) + *def_stmt = STMT_VINFO_RELATED_STMT (def_vinfo); + } + if (!is_gimple_assign (*def_stmt)) return false; does that fix it? Thanks, Richard. > 2016-02-15 Jakub Jelinek <ja...@redhat.com> > > PR tree-optimization/69820 > * tree-vect-patterns.c (type_conversion_p): Return false if > *orig_type is unsigned single precision or boolean. > (vect_recog_dot_prod_pattern, vect_recog_widen_mult_pattern): > Formatting fix. > > * gcc.dg/vect/pr69820.c: New test. > > --- gcc/tree-vect-patterns.c.jj 2016-01-12 14:14:56.000000000 +0100 > +++ gcc/tree-vect-patterns.c 2016-02-15 18:52:52.695249972 +0100 > @@ -184,6 +184,13 @@ type_conversion_p (tree name, gimple *us > || ((TYPE_UNSIGNED (type) != TYPE_UNSIGNED (*orig_type)) && > check_sign)) > return false; > > + /* Conversion from bool or unsigned single bit precision bitfields > + should have been recognized by vect_recog_bool_pattern, callers > + of this function are generally unprepared to handle those. */ > + if ((TYPE_PRECISION (*orig_type) == 1 && TYPE_UNSIGNED (*orig_type)) > + || TREE_CODE (*orig_type) == BOOLEAN_TYPE) > + return false; > + > if (TYPE_PRECISION (type) >= (TYPE_PRECISION (*orig_type) * 2)) > *promotion = true; > else > @@ -334,8 +341,8 @@ vect_recog_dot_prod_pattern (vec<gimple > stmt = last_stmt; > > if (type_conversion_p (oprnd0, stmt, true, &half_type, &def_stmt, > - &promotion) > - && promotion) > + &promotion) > + && promotion) > { > stmt = def_stmt; > oprnd0 = gimple_assign_rhs1 (stmt); > @@ -395,13 +402,13 @@ vect_recog_dot_prod_pattern (vec<gimple > || !types_compatible_p (TREE_TYPE (oprnd1), prod_type)) > return NULL; > if (!type_conversion_p (oprnd0, stmt, true, &half_type0, &def_stmt, > - &promotion) > - || !promotion) > + &promotion) > + || !promotion) > return NULL; > oprnd00 = gimple_assign_rhs1 (def_stmt); > if (!type_conversion_p (oprnd1, stmt, true, &half_type1, &def_stmt, > - &promotion) > - || !promotion) > + &promotion) > + || !promotion) > return NULL; > oprnd01 = gimple_assign_rhs1 (def_stmt); > if (!types_compatible_p (half_type0, half_type1)) > @@ -891,10 +898,10 @@ vect_recog_widen_mult_pattern (vec<gimpl > oprnd = &oprnd1; > } > > - tree old_oprnd = gimple_assign_rhs1 (def_stmt); > - tree new_oprnd = make_ssa_name (half_type0); > - new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd); > - *oprnd = new_oprnd; > + tree old_oprnd = gimple_assign_rhs1 (def_stmt); > + tree new_oprnd = make_ssa_name (half_type0); > + new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd); > + *oprnd = new_oprnd; > } > > /* Handle unsigned case. Look for > --- gcc/testsuite/gcc.dg/vect/pr69820.c.jj 2016-02-15 18:54:15.333122283 > +0100 > +++ gcc/testsuite/gcc.dg/vect/pr69820.c 2016-02-15 18:55:45.529891445 > +0100 > @@ -0,0 +1,35 @@ > +/* PR tree-optimization/69820 */ > + > +#include "tree-vect.h" > + > +unsigned int a[100]; > +long long int b[100]; > +unsigned short c[100]; > + > +__attribute__((noinline, noclone)) void > +foo (void) > +{ > + int i; > + for (i = 0; i < 100; ++i) > + b[i] = a[i] * (c[i] * (_Bool) c[i]); > +} > + > +int > +main () > +{ > + int i; > + if (__SIZEOF_INT__ * __CHAR_BIT__ != 32) > + return 0; > + check_vect (); > + for (i = 0; i < 100; ++i) > + { > + a[i] = 3489456818U; > + b[i] = 0x1eadbeefbeefdeadLL; > + c[i] = 38364; > + } > + foo (); > + for (i = 0; i < 100; ++i) > + if (b[i] != 0xed446af8U) > + __builtin_abort (); > + return 0; > +} > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)