On Thu, Apr 28, 2011 at 9:50 PM, Diego Novillo <dnovi...@google.com> wrote:
>
> This patch from Silvius Rus adds a new flag (-fstrict-enum-precision).
> While porting the patch to 4.6, I noticed that the C++ FE now has a
> similar flag that seems to have similar semantics (-fstrict-enums).
>
> Silvius's patch is used to disable some switch() optimizations that
> assume enum types can *only* take values within that enum (Silvius,
> please correct me if I got this wrong).
>
> Jason, your -fstrict-enums only applies to the C++ front end.
> Silvius's variant affects VRP and gimplification of switches. Would
> it be better if we simply moved -fstrict-enums to common options and
> used that to decide whether to optimize switches in VRP and the
> gimplifier?
I think only the C++ frontend suffers from the "problem". See also
the patch to disable use of this kind of info from VRP.
Richard.
> We use it internally to disable this optimization for code that
> generates values outside of enum ranges.
>
> Committed to google/main. Jason, Silvius, what do you think would be
> the best approach to merge this into trunk?
>
> Thanks. Diego.
>
> 2011-04-27 Silvius Rus <silvius....@gmail.com>
>
> * doc/invoke.texi (fno-strict-enum-precision): Document.
> * gimplify.c (gimplify_switch_expr): If
> -fno-strict-enum-precision is given, do not consider enum
> types.
> * tree-vrp.c (stmt_interesting_for_vrp): If
> -fno-strict-enum-precision is given, do not look at enum
> types.
>
> 2011-04-27 Silvius Rus <silvius....@gmail.com>
>
> * g++.dg/other/no-strict-enum-precision-1.C: New.
> * g++.dg/other/no-strict-enum-precision-2.C: New.
> * g++.dg/other/no-strict-enum-precision-3.C: New.
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index cb04d91..cffc70d 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -393,8 +393,8 @@ Objective-C and Objective-C++ Dialects}.
> -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol
> -fsignaling-nans -fsingle-precision-constant -fsplit-ivs-in-unroller @gol
> -fsplit-wide-types -fstack-protector -fstack-protector-all @gol
> --fstrict-aliasing -fstrict-overflow -fthread-jumps -ftracer @gol
> --ftree-bit-ccp @gol
> +-fstrict-aliasing -fstrict-overflow -fno-strict-enum-precision -fthread-jumps
> +-ftracer -ftree-bit-ccp @gol
> -ftree-builtin-call-dce -ftree-ccp -ftree-ch -ftree-copy-prop @gol
> -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol
> -ftree-forwprop -ftree-fre -ftree-loop-if-convert @gol
> @@ -2073,6 +2073,11 @@ represented in the minimum number of bits needed to
> represent all the
> enumerators). This assumption may not be valid if the program uses a
> cast to convert an arbitrary integer value to the enumeration type.
>
> +@item -fno-strict-enum-precision
> +@opindex fno-strict-enum-precision
> +Do not perform optimizations of switch() statements based on the
> +precision of enum types.
> +
> @item -ftemplate-depth=@var{n}
> @opindex ftemplate-depth
> Set the maximum instantiation depth for template classes to @var{n}.
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 83eaedc..8984d39 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -1602,6 +1602,8 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
> type = TREE_TYPE (SWITCH_COND (switch_expr));
> if (len
> && INTEGRAL_TYPE_P (type)
> + && (flag_strict_enum_precision
> + || TREE_CODE (type) != ENUMERAL_TYPE)
> && TYPE_MIN_VALUE (type)
> && TYPE_MAX_VALUE (type)
> && tree_int_cst_equal (CASE_LOW (VEC_index (tree, labels, 0)),
> diff --git a/gcc/testsuite/g++.dg/other/no-strict-enum-precision-1.C
> b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-1.C
> new file mode 100644
> index 0000000..87f263c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-1.C
> @@ -0,0 +1,22 @@
> +/* { dg-do run } */
> +/* { dg-options "-fno-strict-enum-precision" } */
> +
> +enum zero_one { zero = 0, one = 1 };
> +
> +int* allocate_bool(zero_one e) {
> + int* v = 0;
> + switch (e) {
> + case zero: v = new int(0);
> + case one: v = new int(1);
> + }
> + return v;
> +}
> +
> +int main() {
> + if (allocate_bool(static_cast<zero_one>(999))) {
> + /* Error: should not have matched any case label. */
> + return 1;
> + } else {
> + return 0;
> + }
> +}
> diff --git a/gcc/testsuite/g++.dg/other/no-strict-enum-precision-2.C
> b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-2.C
> new file mode 100644
> index 0000000..5b6af17
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-2.C
> @@ -0,0 +1,26 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-strict-enum-precision" } */
> +
> +enum X {
> + X1,
> + X2
> +};
> +
> +int foo (enum X x) {
> + switch (x) {
> + case X1:
> + return 0;
> + case X2:
> + return 1;
> + }
> + return x;
> +}
> +
> +int main(int argc, char *argv[]) {
> + int n = argc + 999;
> + if (n == foo(static_cast<X>(n))) {
> + return 0;
> + } else {
> + return 1;
> + }
> +}
> diff --git a/gcc/testsuite/g++.dg/other/no-strict-enum-precision-3.C
> b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-3.C
> new file mode 100755
> index 0000000..c3802a8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-3.C
> @@ -0,0 +1,14 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-strict-enum-precision" } */
> +
> +enum X {
> + X1,
> + X2
> +};
> +
> +int main(int argc, char *argv[]) {
> + X x = static_cast<X>(argc + 999);
> + if (x == X1) return 1;
> + if (x == X2) return 1;
> + return 0;
> +}
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 65d249f..d828a8d 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -5553,7 +5553,9 @@ stmt_interesting_for_vrp (gimple stmt)
> && ((is_gimple_call (stmt)
> && gimple_call_fndecl (stmt) != NULL_TREE
> && DECL_IS_BUILTIN (gimple_call_fndecl (stmt)))
> - || !gimple_vuse (stmt)))
> + || !gimple_vuse (stmt))
> + && (flag_strict_enum_precision
> + || TREE_CODE (TREE_TYPE (lhs)) != ENUMERAL_TYPE))
> return true;
> }
> else if (gimple_code (stmt) == GIMPLE_COND
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4433083
>
