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?
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
>>>>>>>>
>>>>>>>
