Re: [google] Add -fstrict-enum-precision flag (issue4433083)
On Tue, May 3, 2011 at 8:34 PM, Jason Merrill ja...@redhat.com wrote: On 04/28/2011 03:50 PM, Diego Novillo 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? We use it internally to disable this optimization for code that generates values outside of enum ranges. It seems that to me that this patch changes optimizations to not believe the lies of the C++ front end, whereas my patch changes the front end to not lie in the first place, making this patch unnecessary. Correct. Richard.
Re: [google] Add -fstrict-enum-precision flag (issue4433083)
On 04/28/2011 03:50 PM, Diego Novillo 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? We use it internally to disable this optimization for code that generates values outside of enum ranges. It seems that to me that this patch changes optimizations to not believe the lies of the C++ front end, whereas my patch changes the front end to not lie in the first place, making this patch unnecessary. Jason
[google] Add -fstrict-enum-precision flag (issue4433083)
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?
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 000..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_castzero_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 000..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_castX(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 000..c3802a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/no-strict-enum-precision-3.C
@@ -0,0 +1,14 @@
+/* { dg-do run }
Re: [google] Add -fstrict-enum-precision flag (issue4433083)
On Thu, Apr 28, 2011 at 03:50:45PM -0400, Diego Novillo wrote: Committed to google/main. Jason, Silvius, what do you think would be the best approach to merge this into trunk? When this code does get merged to trunk, can the testcases abort() on failure rather than returning 1? This is friendlier for embedded target testing. -Nathan
Re: [google] Add -fstrict-enum-precision flag (issue4433083)
... are the testcases formatted according to the GNU guidelines, in terms, for example, of open and closed curly braces? I don't think so, I see some weird (sorry, after all those years unavoidably look to me *really* weird) open braces ending lines? Paolo
Re: [google] Add -fstrict-enum-precision flag (issue4433083)
On Thu, Apr 28, 2011 at 15:57, Paolo Carlini pcarl...@gmail.com wrote: ... are the testcases formatted according to the GNU guidelines They weren't. I've run indent -gnu on all of them. Thanks for noticing. Diego.
Re: [google] Add -fstrict-enum-precision flag (issue4433083)
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 000..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_castzero_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 000..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_castX(n))) {
+ return 0;
+
