Thanks Richard for comments. Will commit it with that change if no surprise from test suite.
Pan -----Original Message----- From: Richard Biener <richard.guent...@gmail.com> Sent: Thursday, September 19, 2024 2:23 PM To: Li, Pan2 <pan2...@intel.com> Cc: gcc-patches@gcc.gnu.org; tamar.christ...@arm.com; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; jeffreya...@gmail.com; rdapp....@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...@intel.com> wrote: > > From: Pan Li <pan2...@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...@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 >