On Fri, Dec 8, 2017 at 1:18 AM, Jeff Law <l...@redhat.com> wrote:
>
> So the underlying issue here is quite simple. Given something like
>
> x = (cond) ? res1 : res2;
>
> EVRP analysis will compute the resultant range using vrp_meet of the
> ranges for res1 and res2. Seems pretty natural.
>
> vrp_meet makes optimistic assumptions if either range is VR_UNDEFINED
> and will set the resultant range to the range of the other operand.
>
> Some callers explicitly mention this is the desired behavior (PHI
> processing). Other callers avoid calling vrp_meet when one of the
> ranges is VR_UNDEFINED and do something sensible
> (extract_range_from_unary_expr, extract_range_from_binary_expr_1).
Note that "those" simply optimize the VR_UNDEFINED case by ignoring
the range. They basically take a shortcut around vrp_meet and avoid
calling extract_range_* on VR_UNDEFINED ranges.
> extract_range_from_cond_expr neither mentions that it wants the
> optimistic behavior nor does it avoid calling vrp_meet with a
> VR_UNDEFINED range. It naturally seems to fit in better with the other
> extract_range_from_* routines.
>
> I'm not at all familiar with the ipa-cp bits, but from a quick look they
> also seems to fall into the extract_* camp.
>
>
> Anyway, normally in a domwalk the only place where we're going to see
> VR_UNDEFINED would be in the PHI nodes. It's one of the nice properties
> of a domwalk :-)
>
> However, for jump threading we look past the dominance frontier;
> furthermore, we do not currently record ranges for statements we process
> as part of the jump threading. But we do try to extract the range each
> statement generates -- we're primarily looking for cases where the
> statement generates a singleton range.
>
> While the plan does include recording ranges as we look past the
> dominance frontier, I strongly believe some serious code cleanup in DOM
> and jump threading needs to happen first. So I don't want to go down
> that path for gcc-8.
>
> So we're kind-of stuck with the fact that we might query for a resultant
> range when one or more input operands may not have recorded range
> information. Thankfully that's easily resolved by making
> extract_range_from_cond_expr work like the other range extraction
> routines and avoid calling vrp_meet when one or more operands is
> VR_UNDEFINED.
>
> Bootstrapped and regression tested on x86_64. OK for the trunk?
>
> Jeff
>
>
> PR tree-optimization/83298
> * vr-values.c (vr_values::extract_range_from_cond_expr): Do not
> call vrp_meet if one of the input operands is VR_UNDEFINED.
>
> PR tree-optimization/83298
> * gcc.c-torture/execute/pr83298.c: New test.
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr83298.c
> b/gcc/testsuite/gcc.c-torture/execute/pr83298.c
> new file mode 100644
> index 00000000000..0e51ababf5c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr83298.c
> @@ -0,0 +1,11 @@
> +
> +int a, b, c = 1;
> +
> +int main ()
> +{
> + for (; b < 1; b++)
> + ;
> + if (!(c * (a < 1)))
> + __builtin_abort ();
> + return 0;
> +}
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 9352e120d9d..ee5ae3c6a27 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -912,6 +912,23 @@ vr_values::extract_range_from_cond_expr (value_range
> *vr, gassign *stmt)
> else
> set_value_range_to_varying (&vr1);
>
> + /* If either range is VR_UNDEFINED, vrp_meet will make the optimistic
> + choice and use the range of the other operand as the result range.
> +
> + Other users of vrp_meet either explicitly filter the calls for
> + this case, or they do not care (PHI processing where unexecutable
> + edges are explicitly expected to be ignored).
> +
> + Like most other callers, we can not generally tolerate the optimistic
> + choice here. Consider jump threading where we're looking into a
> + non-dominated block and thus may not necessarily have processed the
> + ranges for statements within that non-dominated block. */
> + if (vr0.type == VR_UNDEFINED || vr1.type == VR_UNDEFINED)
> + {
> + set_value_range_to_varying (vr);
> + return;
> + }
> +
> /* The resulting value range is the union of the operand ranges */
> copy_value_range (vr, &vr0);
> vrp_meet (vr, &vr1);
>
