[v5,2/4] Genmatch: Refine the gen_phi_on_cond by match_cond_with_binary_phi
Checks
Commit Message
From: Pan Li <pan2.li@intel.com>
This patch would like to leverage the match_cond_with_binary_phi to
match the phi on cond, and get the true/false arg if matched. This
helps a lot to simplify the implementation of gen_phi_on_cond.
Before this patch:
basic_block _b1 = gimple_bb (_a1);
if (gimple_phi_num_args (_a1) == 2)
{
basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
basic_block _db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_0_1 : _pb_1_1;
basic_block _other_db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_1_1 : _pb_0_1;
gcond *_ct_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_1));
if (_ct_1 && EDGE_COUNT (_other_db_1->preds) == 1
&& EDGE_COUNT (_other_db_1->succs) == 1
&& EDGE_PRED (_other_db_1, 0)->src == _db_1)
{
tree _cond_lhs_1 = gimple_cond_lhs (_ct_1);
tree _cond_rhs_1 = gimple_cond_rhs (_ct_1);
tree _p0 = build2 (gimple_cond_code (_ct_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
bool _arg_0_is_true_1 = gimple_phi_arg_edge (_a1, 0)->flags & EDGE_TRUE_VALUE;
tree _p1 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 0 : 1);
tree _p2 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 1 : 0);
...
After this patch:
basic_block _b1 = gimple_bb (_a1);
tree _p1, _p2;
gcond *_cond_1 = match_cond_with_binary_phi (_a1, &_p1, &_p2);
if (_cond_1 && _p1 && _p2)
{
tree _cond_lhs_1 = gimple_cond_lhs (_cond_1);
tree _cond_rhs_1 = gimple_cond_rhs (_cond_1);
tree _p0 = build2 (gimple_cond_code (_cond_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
...
The below test suites are passed for this patch.
* The rv64gcv fully regression test.
* The x86 bootstrap test.
* The x86 fully regression test.
gcc/ChangeLog:
* genmatch.cc (dt_operand::gen_phi_on_cond): Leverage the
match_cond_with_binary_phi API to get cond gimple, true and
false TREE arg.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/genmatch.cc | 67 +++++++++++--------------------------------------
1 file changed, 15 insertions(+), 52 deletions(-)
Comments
On Thu, Sep 19, 2024 at 6:11 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to leverage the match_cond_with_binary_phi to
> match the phi on cond, and get the true/false arg if matched. This
> helps a lot to simplify the implementation of gen_phi_on_cond.
>
> Before this patch:
> basic_block _b1 = gimple_bb (_a1);
> if (gimple_phi_num_args (_a1) == 2)
> {
> basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
> basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
> basic_block _db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_0_1 : _pb_1_1;
> basic_block _other_db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_1_1 : _pb_0_1;
> gcond *_ct_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_1));
> if (_ct_1 && EDGE_COUNT (_other_db_1->preds) == 1
> && EDGE_COUNT (_other_db_1->succs) == 1
> && EDGE_PRED (_other_db_1, 0)->src == _db_1)
> {
> tree _cond_lhs_1 = gimple_cond_lhs (_ct_1);
> tree _cond_rhs_1 = gimple_cond_rhs (_ct_1);
> tree _p0 = build2 (gimple_cond_code (_ct_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
> bool _arg_0_is_true_1 = gimple_phi_arg_edge (_a1, 0)->flags & EDGE_TRUE_VALUE;
> tree _p1 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 0 : 1);
> tree _p2 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 1 : 0);
> ...
>
> After this patch:
> basic_block _b1 = gimple_bb (_a1);
> tree _p1, _p2;
> gcond *_cond_1 = match_cond_with_binary_phi (_a1, &_p1, &_p2);
> if (_cond_1 && _p1 && _p2)
It should be enough to test _cond_1 for nullptr, at least I think the API should
guarantee that _p1 and _p2 are then set correctly.
OK with that change.
Richard.
> {
> tree _cond_lhs_1 = gimple_cond_lhs (_cond_1);
> tree _cond_rhs_1 = gimple_cond_rhs (_cond_1);
> tree _p0 = build2 (gimple_cond_code (_cond_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
> ...
>
> The below test suites are passed for this patch.
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> gcc/ChangeLog:
>
> * genmatch.cc (dt_operand::gen_phi_on_cond): Leverage the
> match_cond_with_binary_phi API to get cond gimple, true and
> false TREE arg.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/genmatch.cc | 67 +++++++++++--------------------------------------
> 1 file changed, 15 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index f1ff1d18265..149458fffe1 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3516,79 +3516,42 @@ dt_operand::gen (FILE *f, int indent, bool gimple, int depth)
> void
> dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> {
> - fprintf_indent (f, indent,
> - "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
> -
> - fprintf_indent (f, indent, "if (gimple_phi_num_args (_a%d) == 2)\n", depth);
> + char opname_0[20];
> + char opname_1[20];
> + char opname_2[20];
>
> - indent += 2;
> - fprintf_indent (f, indent, "{\n");
> - indent += 2;
> + gen_opname (opname_0, 0);
> + gen_opname (opname_1, 1);
> + gen_opname (opname_2, 2);
>
> fprintf_indent (f, indent,
> - "basic_block _pb_0_%d = EDGE_PRED (_b%d, 0)->src;\n", depth, depth);
> - fprintf_indent (f, indent,
> - "basic_block _pb_1_%d = EDGE_PRED (_b%d, 1)->src;\n", depth, depth);
> - fprintf_indent (f, indent,
> - "basic_block _db_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%d)) ? "
> - "_pb_0_%d : _pb_1_%d;\n", depth, depth, depth, depth);
> + "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
> + fprintf_indent (f, indent, "tree %s, %s;\n", opname_1, opname_2);
> fprintf_indent (f, indent,
> - "basic_block _other_db_%d = safe_dyn_cast <gcond *> "
> - "(*gsi_last_bb (_pb_0_%d)) ? _pb_1_%d : _pb_0_%d;\n",
> - depth, depth, depth, depth);
> + "gcond *_cond_%d = match_cond_with_binary_phi (_a%d, &%s, &%s);\n",
> + depth, depth, opname_1, opname_2);
>
> - fprintf_indent (f, indent,
> - "gcond *_ct_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_%d));\n",
> - depth, depth);
> - fprintf_indent (f, indent, "if (_ct_%d"
> - " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> - fprintf_indent (f, indent,
> - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> - fprintf_indent (f, indent,
> - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> + fprintf_indent (f, indent, "if (_cond_%d && %s && %s)\n",
> + depth, opname_1, opname_2);
>
> indent += 2;
> fprintf_indent (f, indent, "{\n");
> indent += 2;
>
> fprintf_indent (f, indent,
> - "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> + "tree _cond_lhs_%d = gimple_cond_lhs (_cond_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> - "tree _cond_rhs_%d = gimple_cond_rhs (_ct_%d);\n", depth, depth);
> -
> - char opname_0[20];
> - char opname_1[20];
> - char opname_2[20];
> - gen_opname (opname_0, 0);
> -
> + "tree _cond_rhs_%d = gimple_cond_rhs (_cond_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> - "tree %s = build2 (gimple_cond_code (_ct_%d), "
> + "tree %s = build2 (gimple_cond_code (_cond_%d), "
> "boolean_type_node, _cond_lhs_%d, _cond_rhs_%d);\n",
> opname_0, depth, depth, depth);
>
> - fprintf_indent (f, indent,
> - "bool _arg_0_is_true_%d = gimple_phi_arg_edge (_a%d, 0)->flags"
> - " & EDGE_TRUE_VALUE;\n", depth, depth);
> -
> - gen_opname (opname_1, 1);
> - fprintf_indent (f, indent,
> - "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 0 : 1);\n",
> - opname_1, depth, depth);
> -
> - gen_opname (opname_2, 2);
> - fprintf_indent (f, indent,
> - "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 1 : 0);\n",
> - opname_2, depth, depth);
> -
> gen_kids (f, indent, true, depth);
>
> indent -= 2;
> fprintf_indent (f, indent, "}\n");
> indent -= 2;
> -
> - indent -= 2;
> - fprintf_indent (f, indent, "}\n");
> - indent -= 2;
> }
>
> /* Emit a logging call to the debug file to the file F, with the INDENT from
> --
> 2.43.0
>
Thanks Richard for comments.
Will commit it with that change if no surprise from test suite.
Pan
-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com>
Sent: Thursday, September 19, 2024 2:23 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; Tamar.Christina@arm.com; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com
Subject: Re: [PATCH v5 2/4] Genmatch: Refine the gen_phi_on_cond by match_cond_with_binary_phi
On Thu, Sep 19, 2024 at 6:11 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to leverage the match_cond_with_binary_phi to
> match the phi on cond, and get the true/false arg if matched. This
> helps a lot to simplify the implementation of gen_phi_on_cond.
>
> Before this patch:
> basic_block _b1 = gimple_bb (_a1);
> if (gimple_phi_num_args (_a1) == 2)
> {
> basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
> basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
> basic_block _db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_0_1 : _pb_1_1;
> basic_block _other_db_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1)) ? _pb_1_1 : _pb_0_1;
> gcond *_ct_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_1));
> if (_ct_1 && EDGE_COUNT (_other_db_1->preds) == 1
> && EDGE_COUNT (_other_db_1->succs) == 1
> && EDGE_PRED (_other_db_1, 0)->src == _db_1)
> {
> tree _cond_lhs_1 = gimple_cond_lhs (_ct_1);
> tree _cond_rhs_1 = gimple_cond_rhs (_ct_1);
> tree _p0 = build2 (gimple_cond_code (_ct_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
> bool _arg_0_is_true_1 = gimple_phi_arg_edge (_a1, 0)->flags & EDGE_TRUE_VALUE;
> tree _p1 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 0 : 1);
> tree _p2 = gimple_phi_arg_def (_a1, _arg_0_is_true_1 ? 1 : 0);
> ...
>
> After this patch:
> basic_block _b1 = gimple_bb (_a1);
> tree _p1, _p2;
> gcond *_cond_1 = match_cond_with_binary_phi (_a1, &_p1, &_p2);
> if (_cond_1 && _p1 && _p2)
It should be enough to test _cond_1 for nullptr, at least I think the API should
guarantee that _p1 and _p2 are then set correctly.
OK with that change.
Richard.
> {
> tree _cond_lhs_1 = gimple_cond_lhs (_cond_1);
> tree _cond_rhs_1 = gimple_cond_rhs (_cond_1);
> tree _p0 = build2 (gimple_cond_code (_cond_1), boolean_type_node, _cond_lhs_1, _cond_rhs_1);
> ...
>
> The below test suites are passed for this patch.
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> gcc/ChangeLog:
>
> * genmatch.cc (dt_operand::gen_phi_on_cond): Leverage the
> match_cond_with_binary_phi API to get cond gimple, true and
> false TREE arg.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/genmatch.cc | 67 +++++++++++--------------------------------------
> 1 file changed, 15 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index f1ff1d18265..149458fffe1 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3516,79 +3516,42 @@ dt_operand::gen (FILE *f, int indent, bool gimple, int depth)
> void
> dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> {
> - fprintf_indent (f, indent,
> - "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
> -
> - fprintf_indent (f, indent, "if (gimple_phi_num_args (_a%d) == 2)\n", depth);
> + char opname_0[20];
> + char opname_1[20];
> + char opname_2[20];
>
> - indent += 2;
> - fprintf_indent (f, indent, "{\n");
> - indent += 2;
> + gen_opname (opname_0, 0);
> + gen_opname (opname_1, 1);
> + gen_opname (opname_2, 2);
>
> fprintf_indent (f, indent,
> - "basic_block _pb_0_%d = EDGE_PRED (_b%d, 0)->src;\n", depth, depth);
> - fprintf_indent (f, indent,
> - "basic_block _pb_1_%d = EDGE_PRED (_b%d, 1)->src;\n", depth, depth);
> - fprintf_indent (f, indent,
> - "basic_block _db_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%d)) ? "
> - "_pb_0_%d : _pb_1_%d;\n", depth, depth, depth, depth);
> + "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
> + fprintf_indent (f, indent, "tree %s, %s;\n", opname_1, opname_2);
> fprintf_indent (f, indent,
> - "basic_block _other_db_%d = safe_dyn_cast <gcond *> "
> - "(*gsi_last_bb (_pb_0_%d)) ? _pb_1_%d : _pb_0_%d;\n",
> - depth, depth, depth, depth);
> + "gcond *_cond_%d = match_cond_with_binary_phi (_a%d, &%s, &%s);\n",
> + depth, depth, opname_1, opname_2);
>
> - fprintf_indent (f, indent,
> - "gcond *_ct_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_%d));\n",
> - depth, depth);
> - fprintf_indent (f, indent, "if (_ct_%d"
> - " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> - fprintf_indent (f, indent,
> - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> - fprintf_indent (f, indent,
> - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> + fprintf_indent (f, indent, "if (_cond_%d && %s && %s)\n",
> + depth, opname_1, opname_2);
>
> indent += 2;
> fprintf_indent (f, indent, "{\n");
> indent += 2;
>
> fprintf_indent (f, indent,
> - "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> + "tree _cond_lhs_%d = gimple_cond_lhs (_cond_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> - "tree _cond_rhs_%d = gimple_cond_rhs (_ct_%d);\n", depth, depth);
> -
> - char opname_0[20];
> - char opname_1[20];
> - char opname_2[20];
> - gen_opname (opname_0, 0);
> -
> + "tree _cond_rhs_%d = gimple_cond_rhs (_cond_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> - "tree %s = build2 (gimple_cond_code (_ct_%d), "
> + "tree %s = build2 (gimple_cond_code (_cond_%d), "
> "boolean_type_node, _cond_lhs_%d, _cond_rhs_%d);\n",
> opname_0, depth, depth, depth);
>
> - fprintf_indent (f, indent,
> - "bool _arg_0_is_true_%d = gimple_phi_arg_edge (_a%d, 0)->flags"
> - " & EDGE_TRUE_VALUE;\n", depth, depth);
> -
> - gen_opname (opname_1, 1);
> - fprintf_indent (f, indent,
> - "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 0 : 1);\n",
> - opname_1, depth, depth);
> -
> - gen_opname (opname_2, 2);
> - fprintf_indent (f, indent,
> - "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 1 : 0);\n",
> - opname_2, depth, depth);
> -
> gen_kids (f, indent, true, depth);
>
> indent -= 2;
> fprintf_indent (f, indent, "}\n");
> indent -= 2;
> -
> - indent -= 2;
> - fprintf_indent (f, indent, "}\n");
> - indent -= 2;
> }
>
> /* Emit a logging call to the debug file to the file F, with the INDENT from
> --
> 2.43.0
>
@@ -3516,79 +3516,42 @@ dt_operand::gen (FILE *f, int indent, bool gimple, int depth)
void
dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
{
- fprintf_indent (f, indent,
- "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
-
- fprintf_indent (f, indent, "if (gimple_phi_num_args (_a%d) == 2)\n", depth);
+ char opname_0[20];
+ char opname_1[20];
+ char opname_2[20];
- indent += 2;
- fprintf_indent (f, indent, "{\n");
- indent += 2;
+ gen_opname (opname_0, 0);
+ gen_opname (opname_1, 1);
+ gen_opname (opname_2, 2);
fprintf_indent (f, indent,
- "basic_block _pb_0_%d = EDGE_PRED (_b%d, 0)->src;\n", depth, depth);
- fprintf_indent (f, indent,
- "basic_block _pb_1_%d = EDGE_PRED (_b%d, 1)->src;\n", depth, depth);
- fprintf_indent (f, indent,
- "basic_block _db_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%d)) ? "
- "_pb_0_%d : _pb_1_%d;\n", depth, depth, depth, depth);
+ "basic_block _b%d = gimple_bb (_a%d);\n", depth, depth);
+ fprintf_indent (f, indent, "tree %s, %s;\n", opname_1, opname_2);
fprintf_indent (f, indent,
- "basic_block _other_db_%d = safe_dyn_cast <gcond *> "
- "(*gsi_last_bb (_pb_0_%d)) ? _pb_1_%d : _pb_0_%d;\n",
- depth, depth, depth, depth);
+ "gcond *_cond_%d = match_cond_with_binary_phi (_a%d, &%s, &%s);\n",
+ depth, depth, opname_1, opname_2);
- fprintf_indent (f, indent,
- "gcond *_ct_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_%d));\n",
- depth, depth);
- fprintf_indent (f, indent, "if (_ct_%d"
- " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
- fprintf_indent (f, indent,
- " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
- fprintf_indent (f, indent,
- " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
+ fprintf_indent (f, indent, "if (_cond_%d && %s && %s)\n",
+ depth, opname_1, opname_2);
indent += 2;
fprintf_indent (f, indent, "{\n");
indent += 2;
fprintf_indent (f, indent,
- "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
+ "tree _cond_lhs_%d = gimple_cond_lhs (_cond_%d);\n", depth, depth);
fprintf_indent (f, indent,
- "tree _cond_rhs_%d = gimple_cond_rhs (_ct_%d);\n", depth, depth);
-
- char opname_0[20];
- char opname_1[20];
- char opname_2[20];
- gen_opname (opname_0, 0);
-
+ "tree _cond_rhs_%d = gimple_cond_rhs (_cond_%d);\n", depth, depth);
fprintf_indent (f, indent,
- "tree %s = build2 (gimple_cond_code (_ct_%d), "
+ "tree %s = build2 (gimple_cond_code (_cond_%d), "
"boolean_type_node, _cond_lhs_%d, _cond_rhs_%d);\n",
opname_0, depth, depth, depth);
- fprintf_indent (f, indent,
- "bool _arg_0_is_true_%d = gimple_phi_arg_edge (_a%d, 0)->flags"
- " & EDGE_TRUE_VALUE;\n", depth, depth);
-
- gen_opname (opname_1, 1);
- fprintf_indent (f, indent,
- "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 0 : 1);\n",
- opname_1, depth, depth);
-
- gen_opname (opname_2, 2);
- fprintf_indent (f, indent,
- "tree %s = gimple_phi_arg_def (_a%d, _arg_0_is_true_%d ? 1 : 0);\n",
- opname_2, depth, depth);
-
gen_kids (f, indent, true, depth);
indent -= 2;
fprintf_indent (f, indent, "}\n");
indent -= 2;
-
- indent -= 2;
- fprintf_indent (f, indent, "}\n");
- indent -= 2;
}
/* Emit a logging call to the debug file to the file F, with the INDENT from