This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix some EVRP stupidness
On October 18, 2018 5:42:56 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>On 10/18/18 8:11 AM, Richard Biener wrote:
>> On Thu, 18 Oct 2018, Richard Biener wrote:
>>
>>>
>>> At some point we decided to not simply intersect all ranges we get
>>> via register_edge_assert_for. Instead we simply register them
>>> in-order. That causes things like replacing [64, +INF] with ~[0,
>0].
>>>
>>> The following patch avoids replacing a range with a larger one
>>> as obvious improvement.
>>>
>>> Compared to assert_expr based VRP we lack the ability to put down
>>> actual assert_exprs and thus multiple SSA names with ranges we
>>> could link via equivalences. In the end we need sth similar,
>>> for example by keeping a stack of active ranges for each SSA name.
>>>
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to
>trunk.
>>
>> Actually not. Needed to update to the new value_range class and
>after
>> that (and its introduction of ->check()) we now ICE during bootstrap
>> with
>>
>> during GIMPLE pass: evrp
>> /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c: In
>> function ‘get_BID128’:
>>
>/space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c:1851:1:
>> internal compiler error: in check, at tree-vrp.c:155
>> 1851 | }
>> | ^
>> 0xf3a8b5 value_range::check()
>> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:155
>> 0xf42424 value_range::value_range(value_range_kind, tree_node*,
>> tree_node*, bitmap_head*)
>> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:110
>> 0xf42424 set_value_range_with_overflow
>> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1422
>> 0xf42424 extract_range_from_binary_expr_1(value_range*, tree_code,
>> tree_node*, value_range const*, value_range const*)
>> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1679
>>
>> for a PLUS_EXPR of [12254, expon_43] and [-1, -1] yielding
>> (temporarily!) [12254, -1] before supposed to be adjusted by the
>> symbolic bound:
>>
>> /* Adjust the range for possible overflow. */
>> set_value_range_with_overflow (*vr, expr_type,
>> wmin, wmax, min_ovf,
>max_ovf);
>> ^^^ ICE
>> if (vr->varying_p ())
>> return;
>>
>> /* Build the symbolic bounds if needed. */
>> min = vr->min ();
>> max = vr->max ();
>> adjust_symbolic_bound (min, code, expr_type,
>> sym_min_op0, sym_min_op1,
>> neg_min_op0, neg_min_op1);
>> adjust_symbolic_bound (max, code, expr_type,
>> sym_max_op0, sym_max_op1,
>> neg_max_op0, neg_max_op1);
>> type = vr->kind ();
>>
>> I think the refactoring that was applied here is simply not suitable
>> because *vr is _not_ necessarily a valid range before the symbolic
>> bounds have been adjusted. A fix would be sth like the following
>> which I am going to test now.
>
>Sounds reasonable.
Doesn't work and miscompiles all over the place.
>Is this PR87640? Because the testcase there is also crashing while
>creating the range right before adjusting the symbolics.
Might be.
I'll poke some more tomorrow unless you beat me to it.
Richard.
>Thanks for looking at this.
>Aldy