On Wed, May 26, 2021 at 4:28 AM Richard Biener <richard.guent...@gmail.com> wrote: > > On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pins...@gmail.com> wrote: > > > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pins...@gmail.com> wrote: > > > > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > > > <bernd.edlin...@hotmail.de> wrote: > > > > > > > > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote: > > > > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches > > > > > <gcc-patches@gcc.gnu.org> wrote: > > > > >> > > > > >> From: Andrew Pinski <apin...@marvell.com> > > > > >> > > > > >> Instead of some of the more manual optimizations inside phi-opt, > > > > >> it would be good idea to do a lot of the heavy lifting inside match > > > > >> and simplify instead. In the process, this moves the three simple > > > > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications. > > > > >> > > > > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions. > > > > >> > > > > >> Differences from V1: > > > > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean > > > > >> types > > > > >> which are not 1 bit precision. > > > > > > > > > > OK. > > > > > > > > > > Thanks, > > > > > Richard. > > > > > > > > > > > > > Hmm, sorry, no luck. > > > > > > > > I think this caused: > > > > > > If anything it is a bad interaction with changes between r12-1046 and > > > r12-1053; I am suspecting a bug in those changes rather than my > > > changes causing the bug. Debugging it right now. > > > > (gdb) p debug_tree(name) > > <ssa_name 0x7ffff6a5cd38 > > type <boolean_type 0x7ffff6b45b28 _Bool public unsigned QI > > size <integer_cst 0x7ffff6b2bdc8 constant 8> > > unit-size <integer_cst 0x7ffff6b2bde0 constant 1> > > align:8 warn_if_not_align:0 symtab:0 alias-set -1 > > canonical-type 0x7ffff6b45b28 precision:1 min <integer_cst > > 0x7ffff6b4a030 0> max <integer_cst 0x7ffff6b4a060 1>> > > > > def_stmt _19 = ~_8; > > version:19> > > > > So what is happening is evrp converted: > > ct_12 = ct_5 + -1; > > Into > > ct_12 = ct_5 == 1 ? 0 : 1; > > (this was done before my patch) > > Note this COND_EXPR is supposed to be combined > with its single use in a GIMPLE_COND ...
I Noticed it was not doing it (before my patch) inside evrp either. > > > And then it gets simplified to: > > _8 = ct_5 == 1; > > _19 = ~_8; > > ct_12 = (int) _19; > > (after my match.pd patch) > > which this one then breaks. I suppose instead of replacing > ct_12 adjusting the GIMPLE_COND directly might be > a better approach ... or not folding the generated COND_EXPR. I was going to try to see where COND_EXPR is created but it is late and there seems to be other issues going on too. For example, the above really should have been converted to: _19 = ct_5 != 1; ct_12 = (int) _19; This might be a gimple-match issue where the conditional part is always emitted without being simplified with the ~ part; COND_EXPR has those kind of issues :). Thanks, Andrew Pinski > > > Which is correct, but the range code is not expecting new SSA names to > > be added ..... > > It looks like the issue was introduced with r12-1048 (Add imports and > > strengthen the export definition in range_def and gori_map). > > I suspect there are other match.pd patterns which would also hit this > > issue where a new ssa name is introduced. > > > > I have no idea how to get this fixed because gimple-range-* is new to > > me; I tried looking into it but calling has_def_chain/get_def_chain > > inside register_dependency seems wrong, maybe Andrew MacLeod can help > > here. > > This happens even with a stage 1 gcc so it is not miscompiling. > > > > Also sorry for the breakage, I was not expecting it this time around > > as I ran bootstrap like three times and I did not expect an > > interaction with other parts of the compiler like this. > > > > Thanks, > > Andrew Pinski > > > > > > > > > > Thanks, > > > Andrew > > > > > > > > > > > > > > > > > > > > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ > > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ > > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem > > > > /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem > > > > /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include -fchecking=1 -c > > > > -g -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes > > > > -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute > > > > -Wno-overlength-strings -pedantic -Wno-long-long -DHAVE_CONFIG_H -I. > > > > -I../../gcc-trunk/fixincludes -I../include > > > > -I../../gcc-trunk/fixincludes/../include > > > > ../../gcc-trunk/fixincludes/fixtests.c > > > > during GIMPLE pass: evrp > > > > ../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’: > > > > ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: > > > > in operator[], at vec.h:890 > > > > 155 | } > > > > | ^ > > > > 0x824622 vec<tree_node*, va_gc, vl_embed>::operator[](unsigned int) > > > > ../../gcc-trunk/gcc/vec.h:890 > > > > 0x8247f0 vec<range_def_chain::rdc, va_heap, > > > > vl_embed>::operator[](unsigned int) > > > > ../../gcc-trunk/gcc/tree.h:3366 > > > > 0x8247f0 vec<range_def_chain::rdc, va_heap, > > > > vl_ptr>::operator[](unsigned int) > > > > ../../gcc-trunk/gcc/vec.h:1461 > > > > 0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, > > > > basic_block_def*) > > > > ../../gcc-trunk/gcc/gimple-range-gori.cc:179 > > > > 0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, > > > > fur_source&) > > > > ../../gcc-trunk/gcc/gimple-range.cc:439 > > > > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, > > > > tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:376 > > > > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, > > > > tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:1067 > > > > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:1097 > > > > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:980 > > > > 0x18637c7 fold_using_range::range_of_range_op(irange&, gimple*, > > > > fur_source&) > > > > ../../gcc-trunk/gcc/gimple-range.cc:431 > > > > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, > > > > tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:376 > > > > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, > > > > tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:1067 > > > > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:1097 > > > > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*) > > > > ../../gcc-trunk/gcc/gimple-range.cc:980 > > > > 0x1149961 range_query::value_of_expr(tree_node*, gimple*) > > > > ../../gcc-trunk/gcc/value-query.cc:86 > > > > 0x1871e51 hybrid_folder::value_of_expr(tree_node*, gimple*) > > > > ../../gcc-trunk/gcc/gimple-ssa-evrp.c:235 > > > > 0xff9573 substitute_and_fold_engine::replace_uses_in(gimple*) > > > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:575 > > > > 0xff988c > > > > substitute_and_fold_dom_walker::before_dom_children(basic_block_def*) > > > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:845 > > > > 0x183921f dom_walker::walk(basic_block_def*) > > > > ../../gcc-trunk/gcc/domwalk.c:309 > > > > 0xff8d15 > > > > substitute_and_fold_engine::substitute_and_fold(basic_block_def*) > > > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:987 > > > > Please submit a full bug report, > > > > with preprocessed source if appropriate. > > > > Please include the complete backtrace with any bug report. > > > > See <https://gcc.gnu.org/bugs/> for instructions. > > > > make[2]: *** [Makefile:76: fixtests.o] Error 1 > > > > make[2]: Leaving directory '/home/ed/gnu/gcc-build/fixincludes' > > > > make[1]: *** [Makefile:3827: all-fixincludes] Error 2 > > > > make[1]: Leaving directory '/home/ed/gnu/gcc-build' > > > > make: *** [Makefile:1011: all] Error 2 > > > > > > > > > > > > Bernd. > > > > > > > > > > > > >> Thanks, > > > > >> Andrew Pinski > > > > >> > > > > >> gcc: > > > > >> * match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, > > > > >> A?+-1:0, > > > > >> A?POW2:0 and A?0:POW2. > > > > >> --- > > > > >> gcc/match.pd | 41 +++++++++++++++++++++++++++++++++++++++++ > > > > >> 1 file changed, 41 insertions(+) > > > > >> > > > > >> diff --git a/gcc/match.pd b/gcc/match.pd > > > > >> index 1fc6b7b1557..ad6b057c56d 100644 > > > > >> --- a/gcc/match.pd > > > > >> +++ b/gcc/match.pd > > > > >> @@ -3711,6 +3711,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > > > >> (if (integer_all_onesp (@1) && integer_zerop (@2)) > > > > >> @0)))) > > > > >> > > > > >> +/* A few simplifications of "a ? CST1 : CST2". */ > > > > >> +/* NOTE: Only do this on gimple as the if-chain-to-switch > > > > >> + optimization depends on the gimple to have if statements in it. > > > > >> */ > > > > >> +#if GIMPLE > > > > >> +(simplify > > > > >> + (cond @0 INTEGER_CST@1 INTEGER_CST@2) > > > > >> + (switch > > > > >> + (if (integer_zerop (@2)) > > > > >> + (switch > > > > >> + /* a ? 1 : 0 -> a if 0 and 1 are integral types. */ > > > > >> + (if (integer_onep (@1)) > > > > >> + (convert (convert:boolean_type_node @0))) > > > > >> + /* a ? -1 : 0 -> -a. */ > > > > >> + (if (integer_all_onesp (@1)) > > > > >> + (negate (convert (convert:boolean_type_node @0)))) > > > > >> + /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */ > > > > >> + (if (!POINTER_TYPE_P (type) && integer_pow2p (@1)) > > > > >> + (with { > > > > >> + tree shift = build_int_cst (integer_type_node, tree_log2 > > > > >> (@1)); > > > > >> + } > > > > >> + (lshift (convert (convert:boolean_type_node @0)) { shift; > > > > >> }))))) > > > > >> + (if (integer_zerop (@1)) > > > > >> + (with { > > > > >> + tree booltrue = constant_boolean_node (true, > > > > >> boolean_type_node); > > > > >> + } > > > > >> + (switch > > > > >> + /* a ? 0 : 1 -> !a. */ > > > > >> + (if (integer_onep (@2)) > > > > >> + (convert (bit_xor (convert:boolean_type_node @0) { booltrue; > > > > >> } ))) > > > > >> + /* a ? -1 : 0 -> -(!a). */ > > > > >> + (if (integer_all_onesp (@2)) > > > > >> + (negate (convert (bit_xor (convert:boolean_type_node @0) { > > > > >> booltrue; } )))) > > > > >> + /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */ > > > > >> + (if (!POINTER_TYPE_P (type) && integer_pow2p (@2)) > > > > >> + (with { > > > > >> + tree shift = build_int_cst (integer_type_node, tree_log2 > > > > >> (@2)); > > > > >> + } > > > > >> + (lshift (convert (bit_xor (convert:boolean_type_node @0) { > > > > >> booltrue; } )) > > > > >> + { shift; })))))))) > > > > >> +#endif > > > > >> + > > > > >> /* Simplification moved from fold_cond_expr_with_comparison. It > > > > >> may also > > > > >> be extended. */ > > > > >> /* This pattern implements two kinds simplification: > > > > >> -- > > > > >> 2.17.1 > > > > >> > > > > >
