On 9/5/19 3:01 PM, Richard Biener wrote:
> On Tue, 16 Jul 2019, Li Jia He wrote:
>
>> Hi,
>>
>> I made some changes based on the recommendations. Would you like to
>> help me to see it again ? Sorry, it took so long time to provide the
>> patch.
>>
>> Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
>> The reason is that I did not found a good way to handle the
>> optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
>> Maybe I missing some important information about match.pd.
>> 2. The gimple_resimplify2 function is not used. Since stmt1,
>> stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
>> question with the value on the stack as the return value ?
>> I may have misunderstood Richard's intention.
>
> Sorry for the delay in reviewing.
>
> Rather than exporting gimple_set_code and gimple_size I'd split
> out a
>
> void
> gimple_init (gimple *, enum gimple_code code, unsigned nops);
>
> from gimple_alloc (changing that to GC allocate not cleared
> memory) doing all of the actual initialization. Then the
> allocation would go like
>
> gimple *stmt1 = (gimple *)XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN,
> 3));
> gimple_init (stmt1, GIMPLE_ASSIGN, 3);
>
> with an overload for gimple_size to account for # of ops.
>
> + /* Allocate SSA names(lhs1) on the stack. */
> + tree lhs1 = XALLOCA (tree_node);
>
> you can use (tree) XALLOCA (tree_ssa_name) here
>
> + /* Call the interface function of match.pd to simplify the expression.
> */
> + tree t = gimple_simplify (code, boolean_type_node, gimple_assign_lhs
> (stmt1),
> + gimple_assign_lhs (stmt2), NULL,
> + follow_all_ssa_edges);
> +
> + if (t)
>
> As I told Martin offline the appropriate function to use is
>
> You'd do
>
> gimple_match_op op (gimple_match_cond::UNCOND, code,
> boolean_type_node, gimple_assign_lhs (stmt1),
> gimple_assign_lhs (stmt2));
> if (op->resimplify (NULL, follow_all_ssa_edges))
> {
> if (gimple_simplified_result_is_gimple_val (res_op))
> .. got a constant or SSA name ..
> else if (res_op->code.is_tree_code ()
> && TREE_CODE_CLASS ((tree_code)res_op->code)) ==
> tcc_comparison)
> ... got a comparison res_op->op[0] res_op->code res_op->op[1] ...
>
> so you get the outermost expression back decomposed.
>
> Otherwise with you passing NULL as the gimple_seq you'll miss quite
> some simplification cases.
>
> You also have to watch out for the result containing the LHS of one
> of your temporary stmts.
>
> + if (tree t = maybe_fold_comparisons_from_match_pd (BIT_AND_EXPR, code1,
> op1a,
> + op1b, code2, op2a,
> op2b))
> + return t;
> +
> + if (tree t = maybe_fold_comparisons_from_match_pd (BIT_AND_EXPR, code2,
> op2a,
> + op2b, code1, op1a,
> op1b))
> + return t;
>
> with match.pd rules you shouldn't need to call this twice, once with
> swapped operands.
>
> Otherwise this first patch looks like what I'd have done and we
> can build upon it.
>
> Not sure if you or Martin wants to improve it according to my
> comments.
>
> Thanks,
> Richard.
>
I'm sending patch that addresses the feedback from Richard.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>From 6f8feb5ccf46bae2c65b88a90661e23521ce9143 Mon Sep 17 00:00:00 2001
From: Li Jia He <heli...@linux.ibm.com>
Date: Mon, 15 Jul 2019 00:30:25 -0500
Subject: [PATCH 1/2] Auto-generate maybe_fold_and/or_comparisons from match.pd
gcc/ChangeLog
2019-07-16 Li Jia He <heli...@linux.ibm.com>
Martin Liska <mli...@suse.cz>
* gimple.h (gimple_init): Declare.
(gimple_size): Likewise.
* gimple.c (gimple_init): Remove static and inline restrictions.
(gimple_alloc): Only allocate memory and call gimple_init.
(gimple_size): Likewise.
* gimple-fold.c (maybe_fold_comparisons_from_match_pd): New function.
(maybe_fold_and_comparisons): Modify and_comparisons_1 invocation and
call maybe_fold_comparisons_from_match_pd.
(maybe_fold_or_comparisons): Modify or_comparisons_1 invocation and
call maybe_fold_comparisons_from_match_pd.
* tree-ssanames.c (init_ssa_name_imm_use): Use make_ssa_name_fn.
(make_ssa_name_fn): New.
* tree-ssanames.h (init_ssa_name_imm_use): New.
---
gcc/gimple-fold.c | 108 ++++++++++++++++++++++++++++++++++++++++----
gcc/gimple.c | 37 +++++++++------
gcc/gimple.h | 2 +
gcc/tree-ssanames.c | 21 ++++++---
gcc/tree-ssanames.h | 1 +
5 files changed, 138 insertions(+), 31 deletions(-)
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index fcffb9802b7..5a42d5bebee 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -5834,6 +5834,85 @@ and_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
return NULL_TREE;
}
+/* Helper function for maybe_fold_and_comparisons and maybe_fold_or_comparisons
+ : try to simplify the AND/OR of the ssa variable VAR with the comparison
+ specified by (OP2A CODE2 OP2B) from match.pd. Return NULL_EXPR if we can't
+ simplify this to a single expression. As we are going to lower the cost
+ of building SSA names / gimple stmts significantly, we need to allocate
+ them ont the stack. This will cause the code to be a bit ugly. */
+
+static tree
+maybe_fold_comparisons_from_match_pd (enum tree_code code, enum tree_code code1,
+ tree op1a, tree op1b,
+ enum tree_code code2, tree op2a,
+ tree op2b)
+{
+ /* Allocate gimple stmt1 on the stack. */
+ gimple *stmt1 = (gimple *) XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN, 2));
+ gimple_init (stmt1, GIMPLE_ASSIGN, 3);
+ gimple_assign_set_rhs_code (stmt1, code1);
+ gimple_assign_set_rhs1 (stmt1, op1a);
+ gimple_assign_set_rhs2 (stmt1, op1b);
+
+ /* Allocate gimple stmt2 on the stack. */
+ gimple *stmt2 = (gimple *) XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN, 2));
+ gimple_init (stmt2, GIMPLE_ASSIGN, 3);
+ gimple_assign_set_rhs_code (stmt2, code2);
+ gimple_assign_set_rhs1 (stmt2, op2a);
+ gimple_assign_set_rhs2 (stmt2, op2b);
+
+ /* Allocate SSA names(lhs1) on the stack. */
+ tree lhs1 = (tree)XALLOCA (tree_ssa_name);
+ memset (lhs1, 0, sizeof (tree_ssa_name));
+ TREE_SET_CODE (lhs1, SSA_NAME);
+ TREE_TYPE (lhs1) = boolean_type_node;
+ init_ssa_name_imm_use (lhs1);
+
+ /* Allocate SSA names(lhs2) on the stack. */
+ tree lhs2 = (tree)XALLOCA (tree_ssa_name);
+ memset (lhs2, 0, sizeof (tree_ssa_name));
+ TREE_SET_CODE (lhs2, SSA_NAME);
+ TREE_TYPE (lhs2) = boolean_type_node;
+ init_ssa_name_imm_use (lhs2);
+
+ gimple_assign_set_lhs (stmt1, lhs1);
+ gimple_assign_set_lhs (stmt2, lhs2);
+
+ gimple_match_op op (gimple_match_cond::UNCOND, code,
+ boolean_type_node, gimple_assign_lhs (stmt1),
+ gimple_assign_lhs (stmt2));
+ if (op.resimplify (NULL, follow_all_ssa_edges))
+ {
+ if (gimple_simplified_result_is_gimple_val (&op))
+ {
+ tree res = op.ops[0];
+ switch (TREE_CODE (res))
+ {
+ case SSA_NAME:
+ {
+ gimple *def = SSA_NAME_DEF_STMT (res);
+
+ if (!is_gimple_assign (def)
+ || TREE_CODE_CLASS (gimple_assign_rhs_code (def))
+ != tcc_comparison)
+ return NULL_TREE;
+
+ return fold_build2 (gimple_assign_rhs_code (def),
+ boolean_type_node, gimple_assign_rhs1 (def),
+ gimple_assign_rhs2 (def));
+ }
+ case INTEGER_CST:
+ /* Fold expression to boolean_true_node or boolean_false_node. */
+ return res;
+ default:
+ return NULL_TREE;
+ }
+ }
+ }
+
+ return NULL_TREE;
+}
+
/* Try to simplify the AND of two comparisons, specified by
(OP1A CODE1 OP1B) and (OP2B CODE2 OP2B), respectively.
If this can be simplified to a single expression (without requiring
@@ -5845,11 +5924,17 @@ tree
maybe_fold_and_comparisons (enum tree_code code1, tree op1a, tree op1b,
enum tree_code code2, tree op2a, tree op2b)
{
- tree t = and_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b);
- if (t)
+ if (tree t = and_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b))
return t;
- else
- return and_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b);
+
+ if (tree t = and_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b))
+ return t;
+
+ if (tree t = maybe_fold_comparisons_from_match_pd (BIT_AND_EXPR, code1, op1a,
+ op1b, code2, op2a, op2b))
+ return t;
+
+ return NULL_TREE;
}
/* Helper function for or_comparisons_1: try to simplify the OR of the
@@ -6309,13 +6394,18 @@ tree
maybe_fold_or_comparisons (enum tree_code code1, tree op1a, tree op1b,
enum tree_code code2, tree op2a, tree op2b)
{
- tree t = or_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b);
- if (t)
+ if (tree t = or_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b))
+ return t;
+
+ if (tree t = or_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b))
return t;
- else
- return or_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b);
-}
+ if (tree t = maybe_fold_comparisons_from_match_pd (BIT_IOR_EXPR, code1, op1a,
+ op1b, code2, op2a, op2b))
+ return t;
+
+ return NULL_TREE;
+}
/* Fold STMT to a constant using VALUEIZE to valueize SSA names.
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 633ef512a19..88250cad16b 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -110,10 +110,27 @@ gimple_set_code (gimple *g, enum gimple_code code)
/* Return the number of bytes needed to hold a GIMPLE statement with
code CODE. */
-static inline size_t
-gimple_size (enum gimple_code code)
+size_t
+gimple_size (enum gimple_code code, unsigned num_ops)
{
- return gsstruct_code_size[gss_for_code (code)];
+ size_t size = gsstruct_code_size[gss_for_code (code)];
+ if (num_ops > 0)
+ size += (sizeof (tree) * (num_ops - 1));
+ return size;
+}
+
+/* Initialize GIMPLE statement G with CODE and NUM_OPS. */
+
+void
+gimple_init (gimple *g, enum gimple_code code, unsigned num_ops)
+{
+ gimple_set_code (g, code);
+ gimple_set_num_ops (g, num_ops);
+
+ /* Do not call gimple_set_modified here as it has other side
+ effects and this tuple is still not completely built. */
+ g->modified = 1;
+ gimple_init_singleton (g);
}
/* Allocate memory for a GIMPLE statement with code CODE and NUM_OPS
@@ -125,10 +142,7 @@ gimple_alloc (enum gimple_code code, unsigned num_ops MEM_STAT_DECL)
size_t size;
gimple *stmt;
- size = gimple_size (code);
- if (num_ops > 0)
- size += sizeof (tree) * (num_ops - 1);
-
+ size = gimple_size (code, num_ops);
if (GATHER_STATISTICS)
{
enum gimple_alloc_kind kind = gimple_alloc_kind (code);
@@ -137,14 +151,7 @@ gimple_alloc (enum gimple_code code, unsigned num_ops MEM_STAT_DECL)
}
stmt = ggc_alloc_cleared_gimple_statement_stat (size PASS_MEM_STAT);
- gimple_set_code (stmt, code);
- gimple_set_num_ops (stmt, num_ops);
-
- /* Do not call gimple_set_modified here as it has other side
- effects and this tuple is still not completely built. */
- stmt->modified = 1;
- gimple_init_singleton (stmt);
-
+ gimple_init (stmt, code, num_ops);
return stmt;
}
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 55f5d0d33d9..cf1f8da5ae2 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1445,6 +1445,8 @@ extern enum gimple_statement_structure_enum const gss_for_code_[];
of comminucating the profile info to the builtin expanders. */
extern gimple *currently_expanding_gimple_stmt;
+size_t gimple_size (enum gimple_code code, unsigned num_ops = 0);
+void gimple_init (gimple *g, enum gimple_code code, unsigned num_ops);
gimple *gimple_alloc (enum gimple_code, unsigned CXX_MEM_STAT_INFO);
greturn *gimple_build_return (tree);
void gimple_call_reset_alias_info (gcall *);
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 3911db9c26e..f7b638dba11 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -252,6 +252,19 @@ flush_ssaname_freelist (void)
vec_safe_truncate (FREE_SSANAMES_QUEUE (cfun), 0);
}
+/* Initialize SSA_NAME_IMM_USE_NODE of a SSA NAME. */
+
+void
+init_ssa_name_imm_use (tree name)
+{
+ use_operand_p imm;
+ imm = &(SSA_NAME_IMM_USE_NODE (name));
+ imm->use = NULL;
+ imm->prev = imm;
+ imm->next = imm;
+ imm->loc.ssa_name = name;
+}
+
/* Return an SSA_NAME node for variable VAR defined in statement STMT
in function FN. STMT may be an empty statement for artificial
references (e.g., default definitions created when a variable is
@@ -263,8 +276,6 @@ make_ssa_name_fn (struct function *fn, tree var, gimple *stmt,
unsigned int version)
{
tree t;
- use_operand_p imm;
-
gcc_assert (VAR_P (var)
|| TREE_CODE (var) == PARM_DECL
|| TREE_CODE (var) == RESULT_DECL
@@ -318,11 +329,7 @@ make_ssa_name_fn (struct function *fn, tree var, gimple *stmt,
SSA_NAME_IN_FREE_LIST (t) = 0;
SSA_NAME_IS_DEFAULT_DEF (t) = 0;
- imm = &(SSA_NAME_IMM_USE_NODE (t));
- imm->use = NULL;
- imm->prev = imm;
- imm->next = imm;
- imm->loc.ssa_name = t;
+ init_ssa_name_imm_use (t);
return t;
}
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 6e6cffbce6a..1a7d0bccdf8 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -82,6 +82,7 @@ extern void fini_ssanames (struct function *);
extern void ssanames_print_statistics (void);
extern tree make_ssa_name_fn (struct function *, tree, gimple *,
unsigned int version = 0);
+extern void init_ssa_name_imm_use (tree);
extern void release_ssa_name_fn (struct function *, tree);
extern bool get_ptr_info_alignment (struct ptr_info_def *, unsigned int *,
unsigned int *);
--
2.23.0
