Message ID | 1621762863-26665-1-git-send-email-apinski@marvell.com |
---|---|
State | New |
Headers | show |
Series | [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations | expand |
On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Andrew Pinski <apinski@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. > 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 >
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 <apinski@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: 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 >> >
On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger <bernd.edlinger@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 <apinski@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. 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 > >> > >
On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > <bernd.edlinger@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 <apinski@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) And then it gets simplified to: _8 = ct_5 == 1; _19 = ~_8; ct_12 = (int) _19; (after my match.pd patch) 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 > > >> > > >
On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > > <bernd.edlinger@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 <apinski@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 ... > 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. > 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 > > > >> > > > >
On Wed, May 26, 2021 at 4:28 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > > > > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > > > <bernd.edlinger@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 <apinski@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 > > > > >> > > > > >
On Wed, May 26, 2021 at 1:37 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, May 26, 2021 at 4:28 AM Richard Biener > <richard.guenther@gmail.com> wrote: > > > > On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > > > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > > > > > > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > > > > <bernd.edlinger@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 <apinski@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. I think it is at most done in forwprop, but even then it likely lacks a fold pattern - we only seem to forward comparisons into GIMPLE_CONDs explicitely, leaving the rest to match.pd patterns. > > > > > 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 :). No, we always re-simplify things, but there might be no (bit_not (eq @0 integer_onep)) simplifier. Oddly enough _Bool foo (int x) { _Bool tem = x == 1; return ~tem; } is simplified to return 1 via matching /* Fold ~X op C as X op' ~C, where op' is the swapped comparison. */ (for cmp (simple_comparison) scmp (swapped_simple_comparison) (simplify (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1) (if (single_use (@2) && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)) (scmp @0 (bit_not @1))))) Richard. > 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 > > > > > >> > > > > > >
On Wed, May 26, 2021 at 1:08 PM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > > <bernd.edlinger@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 <apinski@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) > And then it gets simplified to: > _8 = ct_5 == 1; > _19 = ~_8; > ct_12 = (int) _19; > (after my match.pd patch) > > 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. I'm attaching a source that also exhibits the same ICE. The issue is here: void range_def_chain::register_dependency (tree name, tree dep, basic_block bb) { if (!gimple_range_ssa_p (dep)) return; unsigned v = SSA_NAME_VERSION (name); =>struct rdc &src = m_def_chain[v]; gimple *def_stmt = SSA_NAME_DEF_STMT (dep); unsigned dep_v = SSA_NAME_VERSION (dep); The SSA name is x_19, but the length m_def_chain is 19, so m_def_chain[19] ICEs. Aldy
On 5/26/21 8:05 AM, Richard Biener wrote: > On Wed, May 26, 2021 at 1:37 PM Andrew Pinski <pinskia@gmail.com> wrote: >> On Wed, May 26, 2021 at 4:28 AM Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: >>>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >>>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >>>>> <bernd.edlinger@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 <apinski@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. > I think it is at most done in forwprop, but even then it likely > lacks a fold pattern - we only seem to forward comparisons > into GIMPLE_CONDs explicitely, leaving the rest to > match.pd patterns. > >>>> 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 :). > No, we always re-simplify things, but there might be no > (bit_not (eq @0 integer_onep)) simplifier. > > Oddly enough > > _Bool foo (int x) > { > _Bool tem = x == 1; > return ~tem; > } > > is simplified to return 1 via matching > > /* Fold ~X op C as X op' ~C, where op' is the swapped comparison. */ > (for cmp (simple_comparison) > scmp (swapped_simple_comparison) > (simplify > (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1) > (if (single_use (@2) > && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)) > (scmp @0 (bit_not @1))))) > > Richard. > > Which actually looks enticingly similar to what I'm seeing in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100774. IN that code, Folding statement: _1 = ~b_6; is not turning ~[1,1] into [0,0] but rather leaving it as varying. it eventually fills the values in _1 = 0; if (_1 != 0) goto <bb 4>; [INV but it doesnt simply properly.
On 5/26/21 2:05 PM, Richard Biener wrote: > On Wed, May 26, 2021 at 1:37 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> On Wed, May 26, 2021 at 4:28 AM Richard Biener >> <richard.guenther@gmail.com> wrote: >>> >>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: >>>> >>>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >>>>> >>>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >>>>> <bernd.edlinger@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 <apinski@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. > > I think it is at most done in forwprop, but even then it likely > lacks a fold pattern - we only seem to forward comparisons > into GIMPLE_CONDs explicitely, leaving the rest to > match.pd patterns. > How about this for a quick fix: commit b71621f51bc2819bb7d202efabc17fec5cc92f8f Author: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Wed May 26 18:45:09 2021 +0200 Fix gcc-bootstrap issue ... or at least try to. 2021-05-26 Bernd Edlinger <bernd.edlinger@hotmail.de> * gimple-range-gori.cc (range_def_chain::register_dependency): Resize m_def_chain when needed. diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc index a4c4bf5..722bf5d 100644 --- a/gcc/gimple-range-gori.cc +++ b/gcc/gimple-range-gori.cc @@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree dep, basic_block bb) unsigned v = SSA_NAME_VERSION (name); struct rdc &src = m_def_chain[v]; + if (v >= m_def_chain.length ()) + m_def_chain.safe_grow_cleared (num_ssa_names + 1); gimple *def_stmt = SSA_NAME_DEF_STMT (dep); unsigned dep_v = SSA_NAME_VERSION (dep); bitmap b; Should I push this? Or has anyone a better idea? Thanks Bernd. >>> >>>> 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 :). > > No, we always re-simplify things, but there might be no > (bit_not (eq @0 integer_onep)) simplifier. > > Oddly enough > > _Bool foo (int x) > { > _Bool tem = x == 1; > return ~tem; > } > > is simplified to return 1 via matching > > /* Fold ~X op C as X op' ~C, where op' is the swapped comparison. */ > (for cmp (simple_comparison) > scmp (swapped_simple_comparison) > (simplify > (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1) > (if (single_use (@2) > && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)) > (scmp @0 (bit_not @1))))) > > Richard. > >> 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 >>>>>>>> >>>>>>>
On 5/26/21 7:03 PM, Bernd Edlinger wrote: > On 5/26/21 2:05 PM, Richard Biener wrote: >> On Wed, May 26, 2021 at 1:37 PM Andrew Pinski <pinskia@gmail.com> wrote: >>> >>> On Wed, May 26, 2021 at 4:28 AM Richard Biener >>> <richard.guenther@gmail.com> wrote: >>>> >>>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: >>>>> >>>>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >>>>>> >>>>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >>>>>> <bernd.edlinger@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 <apinski@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. >> >> I think it is at most done in forwprop, but even then it likely >> lacks a fold pattern - we only seem to forward comparisons >> into GIMPLE_CONDs explicitely, leaving the rest to >> match.pd patterns. >> > > How about this for a quick fix: > > commit b71621f51bc2819bb7d202efabc17fec5cc92f8f > Author: Bernd Edlinger <bernd.edlinger@hotmail.de> > Date: Wed May 26 18:45:09 2021 +0200 > > Fix gcc-bootstrap issue > > ... or at least try to. > > 2021-05-26 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * gimple-range-gori.cc (range_def_chain::register_dependency): > Resize m_def_chain when needed. > > diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc > index a4c4bf5..722bf5d 100644 > --- a/gcc/gimple-range-gori.cc > +++ b/gcc/gimple-range-gori.cc > @@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree dep, basic_block bb) > > unsigned v = SSA_NAME_VERSION (name); > struct rdc &src = m_def_chain[v]; > + if (v >= m_def_chain.length ()) > + m_def_chain.safe_grow_cleared (num_ssa_names + 1);> gimple *def_stmt = SSA_NAME_DEF_STMT (dep); > unsigned dep_v = SSA_NAME_VERSION (dep); > bitmap b; > > > Should I push this? > Or has anyone a better idea? > Aehm, I meant of course: --- a/gcc/gimple-range-gori.cc +++ b/gcc/gimple-range-gori.cc @@ -176,6 +176,8 @@ range_def_chain::register_dependency (tree name, tree dep, b return; unsigned v = SSA_NAME_VERSION (name); + if (v >= m_def_chain.length ()) + m_def_chain.safe_grow_cleared (num_ssa_names + 1); struct rdc &src = m_def_chain[v]; gimple *def_stmt = SSA_NAME_DEF_STMT (dep); unsigned dep_v = SSA_NAME_VERSION (dep); > > Thanks > Bernd. > >>>> >>>>> 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 :). >> >> No, we always re-simplify things, but there might be no >> (bit_not (eq @0 integer_onep)) simplifier. >> >> Oddly enough >> >> _Bool foo (int x) >> { >> _Bool tem = x == 1; >> return ~tem; >> } >> >> is simplified to return 1 via matching >> >> /* Fold ~X op C as X op' ~C, where op' is the swapped comparison. */ >> (for cmp (simple_comparison) >> scmp (swapped_simple_comparison) >> (simplify >> (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1) >> (if (single_use (@2) >> && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)) >> (scmp @0 (bit_not @1))))) >> >> Richard. >> >>> 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 >>>>>>>>> >>>>>>>>
On 5/26/21 1:07 PM, Bernd Edlinger wrote: > On 5/26/21 7:03 PM, Bernd Edlinger wrote: >> On 5/26/21 2:05 PM, Richard Biener wrote: >>> On Wed, May 26, 2021 at 1:37 PM Andrew Pinski <pinskia@gmail.com> wrote: >>>> On Wed, May 26, 2021 at 4:28 AM Richard Biener >>>> <richard.guenther@gmail.com> wrote: >>>>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pinskia@gmail.com> wrote: >>>>>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >>>>>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >>>>>>> <bernd.edlinger@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 <apinski@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. >>> I think it is at most done in forwprop, but even then it likely >>> lacks a fold pattern - we only seem to forward comparisons >>> into GIMPLE_CONDs explicitely, leaving the rest to >>> match.pd patterns. >>> >> How about this for a quick fix: >> >> commit b71621f51bc2819bb7d202efabc17fec5cc92f8f >> Author: Bernd Edlinger <bernd.edlinger@hotmail.de> >> Date: Wed May 26 18:45:09 2021 +0200 >> >> Fix gcc-bootstrap issue >> >> ... or at least try to. >> >> 2021-05-26 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * gimple-range-gori.cc (range_def_chain::register_dependency): >> Resize m_def_chain when needed. >> >> diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc >> index a4c4bf5..722bf5d 100644 >> --- a/gcc/gimple-range-gori.cc >> +++ b/gcc/gimple-range-gori.cc >> @@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree dep, basic_block bb) >> >> unsigned v = SSA_NAME_VERSION (name); >> struct rdc &src = m_def_chain[v]; >> + if (v >= m_def_chain.length ()) >> + m_def_chain.safe_grow_cleared (num_ssa_names + 1);> gimple *def_stmt = SSA_NAME_DEF_STMT (dep); >> unsigned dep_v = SSA_NAME_VERSION (dep); >> bitmap b; >> >> >> Should I push this? >> Or has anyone a better idea? >> > Aehm, I meant of course: > > --- a/gcc/gimple-range-gori.cc > +++ b/gcc/gimple-range-gori.cc > @@ -176,6 +176,8 @@ range_def_chain::register_dependency (tree name, tree dep, b > return; > > unsigned v = SSA_NAME_VERSION (name); > + if (v >= m_def_chain.length ()) > + m_def_chain.safe_grow_cleared (num_ssa_names + 1); > struct rdc &src = m_def_chain[v]; > gimple *def_stmt = SSA_NAME_DEF_STMT (dep); > unsigned dep_v = SSA_NAME_VERSION (dep); > > yeah, I was just about to say this :-) when everything was restructured, it seems there is a path to the new register_dependency call from the temporal cache which does not do the range check... all the other calls are gated by a call to has_def_chain() which ensures the vector is big enough. This will be redundant in many cases, but will do for now until I do a perf test and see whether I should so something slightly different. push is OK. Andrew
On 5/26/21 7:07 AM, Andrew Pinski wrote: > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >> <bernd.edlinger@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 <apinski@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) > And then it gets simplified to: > _8 = ct_5 == 1; > _19 = ~_8; > ct_12 = (int) _19; > (after my match.pd patch) > > 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 > > >> Yeah, not your fault, mine. I'm sure it would have shown up very soon elsewhere, you were just the unlucky victim. Sorry about that. Andrew
On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote: > On 5/26/21 7:07 AM, Andrew Pinski wrote: >> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: >>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger >>> <bernd.edlinger@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 <apinski@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) >> And then it gets simplified to: >> _8 = ct_5 == 1; >> _19 = ~_8; >> ct_12 = (int) _19; >> (after my match.pd patch) Yup. I've chased this kind of thing down repeatedly through the years. It's rare, but some transformations from match.pd create new SSA_NAMEs and the various passes need to be prepared to handle that. Jeff
On Fri, May 28, 2021 at 7:21 AM Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote: > > On 5/26/21 7:07 AM, Andrew Pinski wrote: > >> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote: > >>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > >>> <bernd.edlinger@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 <apinski@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) > >> And then it gets simplified to: > >> _8 = ct_5 == 1; > >> _19 = ~_8; > >> ct_12 = (int) _19; > >> (after my match.pd patch) > Yup. I've chased this kind of thing down repeatedly through the years. > It's rare, but some transformations from match.pd create new SSA_NAMEs > and the various passes need to be prepared to handle that. You can suppress that at the expense of some missed simplifications. Using fold_stmt_inplace will, for example, or when passing a NULL seq arguments to the lower-level functions. Richard. > Jeff >
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:
From: Andrew Pinski <apinski@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. 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(+)