On Fri, Aug 5, 2016 at 3:08 PM, Patrick Palka <patr...@parcs.ath.cx> wrote:
> Sometimes the case labels of a switch have a different type than the
> switch operand, e.g. in the test case below, the case labels have type t
> and the switch operand has type int. Because the verifier expects that
> case labels of a switch each have the exact same type, it's important to
> avoid changing their types when truncating their range.
>
> Does this patch look OK to commit after bootstrap + regtesting?
Ok, though the verifier should possibly be made more forgiving and
allow all types_compatible_p.
Richard.
> gcc/ChangeLog:
>
> PR tree-optimization/72810
> * tree-vrp.c (simplify_switch_using_ranges): Avoid changing
> the types of the case labels when truncating.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/72810
> * tree-ssa/vrp110.c: New test.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/vrp110.c | 24 +++++++++++++++++++
> gcc/tree-vrp.c | 43
> +++++++++++++++++++---------------
> 2 files changed, 48 insertions(+), 19 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp110.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp110.c
> b/gcc/testsuite/gcc.dg/tree-ssa/vrp110.c
> new file mode 100644
> index 0000000..34d7eb4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp110.c
> @@ -0,0 +1,24 @@
> +/* { dg-options "-O2" } */
> +
> +extern void foo (void);
> +extern void bar (void);
> +
> +void
> +test (int i)
> +{
> + if (i == 1)
> + return;
> +
> + typedef int t;
> + t j = i;
> + switch (j)
> + {
> + case 1:
> + case 2:
> + foo ();
> + break;
> + case 7:
> + bar ();
> + break;
> + }
> +}
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 7e3b513..2c166db 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -9643,61 +9643,66 @@ simplify_switch_using_ranges (gswitch *stmt)
> tree min_label = gimple_switch_label (stmt, min_idx);
> tree max_label = gimple_switch_label (stmt, max_idx);
>
> + /* Avoid changing the types of the case labels when truncating. */
> + tree case_label_type = TREE_TYPE (CASE_LOW (min_label));
> + tree vr_min = fold_convert (case_label_type, vr->min);
> + tree vr_max = fold_convert (case_label_type, vr->max);
> +
> if (vr->type == VR_RANGE)
> {
> /* If OP's value range is [2,8] and the low label range is
> 0 ... 3, truncate the label's range to 2 .. 3. */
> - if (tree_int_cst_compare (CASE_LOW (min_label), vr->min) < 0
> + if (tree_int_cst_compare (CASE_LOW (min_label), vr_min) < 0
> && CASE_HIGH (min_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (min_label), vr->min) >= 0)
> - CASE_LOW (min_label) = vr->min;
> + && tree_int_cst_compare (CASE_HIGH (min_label), vr_min) >= 0)
> + CASE_LOW (min_label) = vr_min;
>
> /* If OP's value range is [2,8] and the high label range is
> 7 ... 10, truncate the label's range to 7 .. 8. */
> - if (tree_int_cst_compare (CASE_LOW (max_label), vr->max) <= 0
> + if (tree_int_cst_compare (CASE_LOW (max_label), vr_max) <= 0
> && CASE_HIGH (max_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (max_label), vr->max) > 0)
> - CASE_HIGH (max_label) = vr->max;
> + && tree_int_cst_compare (CASE_HIGH (max_label), vr_max) > 0)
> + CASE_HIGH (max_label) = vr_max;
> }
> else if (vr->type == VR_ANTI_RANGE)
> {
> - tree one_cst = build_one_cst (TREE_TYPE (op));
> + tree one_cst = build_one_cst (case_label_type);
>
> if (min_label == max_label)
> {
> /* If OP's value range is ~[7,8] and the label's range is
> 7 ... 10, truncate the label's range to 9 ... 10. */
> - if (tree_int_cst_compare (CASE_LOW (min_label), vr->min) == 0
> + if (tree_int_cst_compare (CASE_LOW (min_label), vr_min) == 0
> && CASE_HIGH (min_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (min_label), vr->max) >
> 0)
> + && tree_int_cst_compare (CASE_HIGH (min_label), vr_max) > 0)
> CASE_LOW (min_label)
> - = int_const_binop (PLUS_EXPR, vr->max, one_cst);
> + = int_const_binop (PLUS_EXPR, vr_max, one_cst);
>
> /* If OP's value range is ~[7,8] and the label's range is
> 5 ... 8, truncate the label's range to 5 ... 6. */
> - if (tree_int_cst_compare (CASE_LOW (min_label), vr->min) < 0
> + if (tree_int_cst_compare (CASE_LOW (min_label), vr_min) < 0
> && CASE_HIGH (min_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (min_label), vr->max) ==
> 0)
> + && tree_int_cst_compare (CASE_HIGH (min_label), vr_max) ==
> 0)
> CASE_HIGH (min_label)
> - = int_const_binop (MINUS_EXPR, vr->min, one_cst);
> + = int_const_binop (MINUS_EXPR, vr_min, one_cst);
> }
> else
> {
> /* If OP's value range is ~[2,8] and the low label range is
> 0 ... 3, truncate the label's range to 0 ... 1. */
> - if (tree_int_cst_compare (CASE_LOW (min_label), vr->min) < 0
> + if (tree_int_cst_compare (CASE_LOW (min_label), vr_min) < 0
> && CASE_HIGH (min_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (min_label), vr->min) >=
> 0)
> + && tree_int_cst_compare (CASE_HIGH (min_label), vr_min) >=
> 0)
> CASE_HIGH (min_label)
> - = int_const_binop (MINUS_EXPR, vr->min, one_cst);
> + = int_const_binop (MINUS_EXPR, vr_min, one_cst);
>
> /* If OP's value range is ~[2,8] and the high label range is
> 7 ... 10, truncate the label's range to 9 ... 10. */
> - if (tree_int_cst_compare (CASE_LOW (max_label), vr->max) <= 0
> + if (tree_int_cst_compare (CASE_LOW (max_label), vr_max) <= 0
> && CASE_HIGH (max_label) != NULL_TREE
> - && tree_int_cst_compare (CASE_HIGH (max_label), vr->max) >
> 0)
> + && tree_int_cst_compare (CASE_HIGH (max_label), vr_max) > 0)
> CASE_LOW (max_label)
> - = int_const_binop (PLUS_EXPR, vr->max, one_cst);
> + = int_const_binop (PLUS_EXPR, vr_max, one_cst);
> }
> }
>
> --
> 2.9.2.614.g990027a
>
