diff mbox series

[PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

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

Commit Message

Li, Pan2 via Gcc-patches May 23, 2021, 9:41 a.m. UTC
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(+)

Comments

Richard Biener May 25, 2021, 2:22 p.m. UTC | #1
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
>
Bernd Edlinger May 26, 2021, 8:28 a.m. UTC | #2
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
>>
>
Andrew Pinski May 26, 2021, 9:01 a.m. UTC | #3
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
> >>
> >
Andrew Pinski May 26, 2021, 11:07 a.m. UTC | #4
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
> > >>
> > >
Richard Biener May 26, 2021, 11:27 a.m. UTC | #5
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
> > > >>
> > > >
Andrew Pinski May 26, 2021, 11:37 a.m. UTC | #6
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
> > > > >>
> > > > >
Richard Biener May 26, 2021, 12:05 p.m. UTC | #7
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
> > > > > >>
> > > > > >
Aldy Hernandez May 26, 2021, 4:54 p.m. UTC | #8
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
Andrew MacLeod May 26, 2021, 4:59 p.m. UTC | #9
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.
Bernd Edlinger May 26, 2021, 5:03 p.m. UTC | #10
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
>>>>>>>>
>>>>>>>
Bernd Edlinger May 26, 2021, 5:07 p.m. UTC | #11
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
>>>>>>>>>
>>>>>>>>
Andrew MacLeod May 26, 2021, 5:15 p.m. UTC | #12
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
Andrew MacLeod May 26, 2021, 5:29 p.m. UTC | #13
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
Jeff Law May 28, 2021, 4:53 a.m. UTC | #14
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
Richard Biener May 28, 2021, 7:16 a.m. UTC | #15
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 mbox series

Patch

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: