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 <pins...@gmail.com> wrote:
>>>
>>> On Wed, May 26, 2021 at 4:28 AM Richard Biener
>>> <richard.guent...@gmail.com> wrote:
>>>>
>>>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski <pins...@gmail.com> wrote:
>>>>>
>>>>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pins...@gmail.com> wrote:
>>>>>>
>>>>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
>>>>>> <bernd.edlin...@hotmail.de> wrote:
>>>>>>>
>>>>>>> On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
>>>>>>>> On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
>>>>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>>>>>
>>>>>>>>> From: Andrew Pinski <apin...@marvell.com>
>>>>>>>>>
>>>>>>>>> Instead of some of the more manual optimizations inside phi-opt,
>>>>>>>>> it would be good idea to do a lot of the heavy lifting inside match
>>>>>>>>> and simplify instead. In the process, this moves the three simple
>>>>>>>>> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
>>>>>>>>>
>>>>>>>>> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
>>>>>>>>>
>>>>>>>>> Differences from V1:
>>>>>>>>> * Use bit_xor 1 instead of bit_not to fix the problem with boolean
>>>>>>>>> types
>>>>>>>>> which are not 1 bit precision.
>>>>>>>>
>>>>>>>> OK.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>
>>>>>>> Hmm, sorry, no luck.
>>>>>>>
>>>>>>> I think this caused:
>>>>>>
>>>>>> If anything it is a bad interaction with changes between r12-1046 and
>>>>>> r12-1053; I am suspecting a bug in those changes rather than my
>>>>>> changes causing the bug. Debugging it right now.
>>>>>
>>>>> (gdb) p debug_tree(name)
>>>>> <ssa_name 0x7ffff6a5cd38
>>>>> type <boolean_type 0x7ffff6b45b28 _Bool public unsigned QI
>>>>> size <integer_cst 0x7ffff6b2bdc8 constant 8>
>>>>> unit-size <integer_cst 0x7ffff6b2bde0 constant 1>
>>>>> align:8 warn_if_not_align:0 symtab:0 alias-set -1
>>>>> canonical-type 0x7ffff6b45b28 precision:1 min <integer_cst
>>>>> 0x7ffff6b4a030 0> max <integer_cst 0x7ffff6b4a060 1>>
>>>>>
>>>>> def_stmt _19 = ~_8;
>>>>> version:19>
>>>>>
>>>>> So what is happening is evrp converted:
>>>>> ct_12 = ct_5 + -1;
>>>>> Into
>>>>> ct_12 = ct_5 == 1 ? 0 : 1;
>>>>> (this was done before my patch)
>>>>
>>>> Note this COND_EXPR is supposed to be combined
>>>> with its single use in a GIMPLE_COND ...
>>>
>>> I Noticed it was not doing it (before my patch) inside evrp either.
>>
>> 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.edlin...@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.edlin...@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
>>>>>>>>>
>>>>>>>>
