* [PATCH] Fix up switchconv for strict enum types (PR tree-optimization/91351)
@ 2019-08-29 9:04 Jakub Jelinek
2019-08-29 9:13 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2019-08-29 9:04 UTC (permalink / raw)
To: Richard Biener, Martin Liška; +Cc: gcc-patches
Hi!
switchconv uses unsigned_type_for to get unsigned type to perform
computations in, which is fine if you just do a comparison in that type or
similar, but not when actually constructing range-like checks or doing
further arithmetics on the type.
The reassoc and fold-const range test optimization has range_check_type
function for that, this patch makes use of that function, which among other
things uses an INTEGER_TYPE instead of enums/booleans and verifies the
INTEGER_TYPE minimum/maximum to make sure it properly wraps around.
One issue is that this function can return NULL on some weird integral
types (perhaps Ada special integers), so we need to punt in that case.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-08-29 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/91351
* tree-cfg.c (generate_range_test): Use range_check_type instead of
unsigned_type_for.
* tree-cfgcleanup.c (convert_single_case_switch): Punt if
range_check_type returns NULL.
* tree-switch-conversion.c (switch_conversion::build_one_array):
Use range_check_type instead of unsigned_type_for, don't perform
linear opt if it returns NULL.
(bit_test_cluster::find_bit_tests): Formatting fix.
(bit_test_cluster::emit): Use range_check_type instead of
unsigned_type_for.
(switch_decision_tree::try_switch_expansion): Punt if range_check_type
returns NULL.
* g++.dg/opt/pr91351.C: New test.
--- gcc/tree-cfg.c.jj 2019-07-29 12:56:38.971248016 +0200
+++ gcc/tree-cfg.c 2019-08-28 19:37:50.843262628 +0200
@@ -9221,7 +9221,7 @@ generate_range_test (basic_block bb, tre
tree *lhs, tree *rhs)
{
tree type = TREE_TYPE (index);
- tree utype = unsigned_type_for (type);
+ tree utype = range_check_type (type);
low = fold_convert (utype, low);
high = fold_convert (utype, high);
--- gcc/tree-cfgcleanup.c.jj 2019-04-24 10:10:22.816535073 +0200
+++ gcc/tree-cfgcleanup.c 2019-08-28 19:39:40.032680646 +0200
@@ -101,6 +101,8 @@ convert_single_case_switch (gswitch *swt
if (high)
{
tree lhs, rhs;
+ if (range_check_type (TREE_TYPE (index)) == NULL_TREE)
+ return false;
generate_range_test (bb, index, low, high, &lhs, &rhs);
cond = gimple_build_cond (LE_EXPR, lhs, rhs, NULL_TREE, NULL_TREE);
}
--- gcc/tree-switch-conversion.c.jj 2019-07-10 15:53:01.148520370 +0200
+++ gcc/tree-switch-conversion.c 2019-08-28 19:45:18.062783134 +0200
@@ -605,7 +605,9 @@ switch_conversion::build_one_array (int
vec<constructor_elt, va_gc> *constructor = m_constructors[num];
wide_int coeff_a, coeff_b;
bool linear_p = contains_linear_function_p (constructor, &coeff_a, &coeff_b);
- if (linear_p)
+ tree type;
+ if (linear_p
+ && (type = range_check_type (TREE_TYPE ((*constructor)[0].value))))
{
if (dump_file && coeff_a.to_uhwi () > 0)
fprintf (dump_file, "Linear transformation with A = %" PRId64
@@ -613,13 +615,12 @@ switch_conversion::build_one_array (int
coeff_b.to_shwi ());
/* We must use type of constructor values. */
- tree t = unsigned_type_for (TREE_TYPE ((*constructor)[0].value));
gimple_seq seq = NULL;
- tree tmp = gimple_convert (&seq, t, m_index_expr);
- tree tmp2 = gimple_build (&seq, MULT_EXPR, t,
- wide_int_to_tree (t, coeff_a), tmp);
- tree tmp3 = gimple_build (&seq, PLUS_EXPR, t, tmp2,
- wide_int_to_tree (t, coeff_b));
+ tree tmp = gimple_convert (&seq, type, m_index_expr);
+ tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
+ wide_int_to_tree (type, coeff_a), tmp);
+ tree tmp3 = gimple_build (&seq, PLUS_EXPR, type, tmp2,
+ wide_int_to_tree (type, coeff_b));
tree tmp4 = gimple_convert (&seq, TREE_TYPE (name), tmp3);
gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
load = gimple_build_assign (name, tmp4);
@@ -1351,7 +1352,7 @@ bit_test_cluster::find_bit_tests (vec<cl
entire));
}
else
- for (int i = end - 1; i >= start; i--)
+ for (int i = end - 1; i >= start; i--)
output.safe_push (clusters[i]);
end = start;
@@ -1484,7 +1485,7 @@ bit_test_cluster::emit (tree index_expr,
unsigned int i, j, k;
unsigned int count;
- tree unsigned_index_type = unsigned_type_for (index_type);
+ tree unsigned_index_type = range_check_type (index_type);
gimple_stmt_iterator gsi;
gassign *shift_stmt;
@@ -1794,7 +1795,8 @@ switch_decision_tree::try_switch_expansi
tree index_type = TREE_TYPE (index_expr);
basic_block bb = gimple_bb (m_switch);
- if (gimple_switch_num_labels (m_switch) == 1)
+ if (gimple_switch_num_labels (m_switch) == 1
+ || range_check_type (index_type) == NULL_TREE)
return false;
/* Find the default case target label. */
--- gcc/testsuite/g++.dg/opt/pr91351.C.jj 2019-08-28 19:53:50.946352281 +0200
+++ gcc/testsuite/g++.dg/opt/pr91351.C 2019-08-28 19:53:30.277651740 +0200
@@ -0,0 +1,38 @@
+// PR tree-optimization/91351
+// { dg-do run }
+// { dg-options "-O2 -fstrict-enums" }
+
+enum E { e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12,
+ e13, e14, e15, e16, e17, e18, e19, e20, e21, e22, e23, e24, e25 };
+
+__attribute__((noipa)) void
+foo ()
+{
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+bar ()
+{
+}
+
+__attribute__((noipa)) void
+baz (E e)
+{
+ switch (e)
+ {
+ case e11:
+ case e12:
+ case e13: foo (); break;
+ case e24: break;
+ case e14:
+ case e15: break;
+ default: bar (); break;
+ }
+}
+
+int
+main ()
+{
+ baz (e3);
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix up switchconv for strict enum types (PR tree-optimization/91351)
2019-08-29 9:04 [PATCH] Fix up switchconv for strict enum types (PR tree-optimization/91351) Jakub Jelinek
@ 2019-08-29 9:13 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2019-08-29 9:13 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Martin Liška, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 6051 bytes --]
On Thu, 29 Aug 2019, Jakub Jelinek wrote:
> Hi!
>
> switchconv uses unsigned_type_for to get unsigned type to perform
> computations in, which is fine if you just do a comparison in that type or
> similar, but not when actually constructing range-like checks or doing
> further arithmetics on the type.
> The reassoc and fold-const range test optimization has range_check_type
> function for that, this patch makes use of that function, which among other
> things uses an INTEGER_TYPE instead of enums/booleans and verifies the
> INTEGER_TYPE minimum/maximum to make sure it properly wraps around.
> One issue is that this function can return NULL on some weird integral
> types (perhaps Ada special integers), so we need to punt in that case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2019-08-29 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/91351
> * tree-cfg.c (generate_range_test): Use range_check_type instead of
> unsigned_type_for.
> * tree-cfgcleanup.c (convert_single_case_switch): Punt if
> range_check_type returns NULL.
> * tree-switch-conversion.c (switch_conversion::build_one_array):
> Use range_check_type instead of unsigned_type_for, don't perform
> linear opt if it returns NULL.
> (bit_test_cluster::find_bit_tests): Formatting fix.
> (bit_test_cluster::emit): Use range_check_type instead of
> unsigned_type_for.
> (switch_decision_tree::try_switch_expansion): Punt if range_check_type
> returns NULL.
>
> * g++.dg/opt/pr91351.C: New test.
>
> --- gcc/tree-cfg.c.jj 2019-07-29 12:56:38.971248016 +0200
> +++ gcc/tree-cfg.c 2019-08-28 19:37:50.843262628 +0200
> @@ -9221,7 +9221,7 @@ generate_range_test (basic_block bb, tre
> tree *lhs, tree *rhs)
> {
> tree type = TREE_TYPE (index);
> - tree utype = unsigned_type_for (type);
> + tree utype = range_check_type (type);
>
> low = fold_convert (utype, low);
> high = fold_convert (utype, high);
> --- gcc/tree-cfgcleanup.c.jj 2019-04-24 10:10:22.816535073 +0200
> +++ gcc/tree-cfgcleanup.c 2019-08-28 19:39:40.032680646 +0200
> @@ -101,6 +101,8 @@ convert_single_case_switch (gswitch *swt
> if (high)
> {
> tree lhs, rhs;
> + if (range_check_type (TREE_TYPE (index)) == NULL_TREE)
> + return false;
> generate_range_test (bb, index, low, high, &lhs, &rhs);
> cond = gimple_build_cond (LE_EXPR, lhs, rhs, NULL_TREE, NULL_TREE);
> }
> --- gcc/tree-switch-conversion.c.jj 2019-07-10 15:53:01.148520370 +0200
> +++ gcc/tree-switch-conversion.c 2019-08-28 19:45:18.062783134 +0200
> @@ -605,7 +605,9 @@ switch_conversion::build_one_array (int
> vec<constructor_elt, va_gc> *constructor = m_constructors[num];
> wide_int coeff_a, coeff_b;
> bool linear_p = contains_linear_function_p (constructor, &coeff_a, &coeff_b);
> - if (linear_p)
> + tree type;
> + if (linear_p
> + && (type = range_check_type (TREE_TYPE ((*constructor)[0].value))))
> {
> if (dump_file && coeff_a.to_uhwi () > 0)
> fprintf (dump_file, "Linear transformation with A = %" PRId64
> @@ -613,13 +615,12 @@ switch_conversion::build_one_array (int
> coeff_b.to_shwi ());
>
> /* We must use type of constructor values. */
> - tree t = unsigned_type_for (TREE_TYPE ((*constructor)[0].value));
> gimple_seq seq = NULL;
> - tree tmp = gimple_convert (&seq, t, m_index_expr);
> - tree tmp2 = gimple_build (&seq, MULT_EXPR, t,
> - wide_int_to_tree (t, coeff_a), tmp);
> - tree tmp3 = gimple_build (&seq, PLUS_EXPR, t, tmp2,
> - wide_int_to_tree (t, coeff_b));
> + tree tmp = gimple_convert (&seq, type, m_index_expr);
> + tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
> + wide_int_to_tree (type, coeff_a), tmp);
> + tree tmp3 = gimple_build (&seq, PLUS_EXPR, type, tmp2,
> + wide_int_to_tree (type, coeff_b));
> tree tmp4 = gimple_convert (&seq, TREE_TYPE (name), tmp3);
> gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> load = gimple_build_assign (name, tmp4);
> @@ -1351,7 +1352,7 @@ bit_test_cluster::find_bit_tests (vec<cl
> entire));
> }
> else
> - for (int i = end - 1; i >= start; i--)
> + for (int i = end - 1; i >= start; i--)
> output.safe_push (clusters[i]);
>
> end = start;
> @@ -1484,7 +1485,7 @@ bit_test_cluster::emit (tree index_expr,
> unsigned int i, j, k;
> unsigned int count;
>
> - tree unsigned_index_type = unsigned_type_for (index_type);
> + tree unsigned_index_type = range_check_type (index_type);
>
> gimple_stmt_iterator gsi;
> gassign *shift_stmt;
> @@ -1794,7 +1795,8 @@ switch_decision_tree::try_switch_expansi
> tree index_type = TREE_TYPE (index_expr);
> basic_block bb = gimple_bb (m_switch);
>
> - if (gimple_switch_num_labels (m_switch) == 1)
> + if (gimple_switch_num_labels (m_switch) == 1
> + || range_check_type (index_type) == NULL_TREE)
> return false;
>
> /* Find the default case target label. */
> --- gcc/testsuite/g++.dg/opt/pr91351.C.jj 2019-08-28 19:53:50.946352281 +0200
> +++ gcc/testsuite/g++.dg/opt/pr91351.C 2019-08-28 19:53:30.277651740 +0200
> @@ -0,0 +1,38 @@
> +// PR tree-optimization/91351
> +// { dg-do run }
> +// { dg-options "-O2 -fstrict-enums" }
> +
> +enum E { e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12,
> + e13, e14, e15, e16, e17, e18, e19, e20, e21, e22, e23, e24, e25 };
> +
> +__attribute__((noipa)) void
> +foo ()
> +{
> + __builtin_abort ();
> +}
> +
> +__attribute__((noipa)) void
> +bar ()
> +{
> +}
> +
> +__attribute__((noipa)) void
> +baz (E e)
> +{
> + switch (e)
> + {
> + case e11:
> + case e12:
> + case e13: foo (); break;
> + case e24: break;
> + case e14:
> + case e15: break;
> + default: bar (); break;
> + }
> +}
> +
> +int
> +main ()
> +{
> + baz (e3);
> +}
>
> Jakub
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG MÌnchen)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-08-29 9:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 9:04 [PATCH] Fix up switchconv for strict enum types (PR tree-optimization/91351) Jakub Jelinek
2019-08-29 9:13 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).