http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59417
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> The problem is the uninitialized var t in the testcase, with it apparently
> the range info weird and inconsistent, but what sanity can one expect from
> uninitialized value, any use of it is invalid.
>
> Before *.copyprop5 we have:
> # RANGE [1, 2147483647] NONZERO 0x0000000007fffffff
> # t_4 = PHI <t_19(D)(3), t_8(23)>
> (supposedly because we ignore the uninitialized var in some spots).
> Then during copyprop5 we copy the range info of t_4 to t_19(D):
> else if (!POINTER_TYPE_P (TREE_TYPE (var))
> && SSA_NAME_RANGE_INFO (var)
> && !SSA_NAME_RANGE_INFO (copy_of[i].value))
> duplicate_ssa_name_range_info (copy_of[i].value,
> SSA_NAME_RANGE_TYPE (var),
> SSA_NAME_RANGE_INFO (var));
> I'm wondering how that can be a safe thing to do, even when not taking into
> account undefined vars. If we have say:
> if (parm_5(D) < 32)
> {
> do_something_with (parm_5(D));
> goto return;
> }
> somevar_21 = parm_5(D);
> and somevar_21 will have range info of [32, +INF] (from VRP ASSERT_EXPRs),
> then
> I don't see how it can be safe to modify parm_5(D)'s SSA_NAME_RANGE_INFO
> (similarly for pointer alignment info though). For this testcase, surely we
> could say avoid doing it if the copy_of[i].value SSA_NAME is default def,
> but I think it is a general issue. Perhaps points to info can be copied
> over, but not alignment info or range info. Maybe we could consider not
> doing copyprop or forwprop replacing one SSA_NAME with another one if the
> one to be replaced has better range info (or alignment info) and only
> copyprop/forwprop if we would replace SSA_NAME with something other than
> SSA_NAME?
Yeah, I wondered about this as well after reviewing the original patches.
If you consider
a_2 = a_1;
if (a_2 > 5)
a_3 = a_2;
then VRP would say a_3 is [6, +INF]. If then copyprop comes along it
sees that a_3 = a_2 = a_1 and would transfer the value-range to a_1.
Note that the loop doing that may replace SSA annotation multiple times
if copy_of[N].value == X for multiple N.
Basically the code relies on the information being not control sensitive.
That's fine for points-to info (but alignment tracking now uses nonzero_bits?),
but for the rest only if var is post-dominated by copy_of[].value's definition.
I don't think we should limit what copyprop/forwprop does though.
Now, why do we arrive at a value-range where we fail that assertion?
> Then in tree-ssa-loop-niter.c I can surely instead of assetion failure just
> give up on using the range info altogether (rtype = VR_VARYING; break;) or
> just using var's range info and nothing else if there is inconsistency
> (rtype = get_range_info (var, &minv, &maxv); break;).
