Thanks Richard for comments.
> Sorry to spoil this again, but can you instead create an interface like
Need mind, let me update it.
> gcond *
> match_cond_with_phi (gphi *phi, tree *true_arg, tree *false_arg);
> That would from a PHI node match up the controlling condition and
> initialize {true,false}_arg with the PHI args that match the conditions
> true/false case?
> I also think for the diamond case you fail to identify the appropriate
> true/false PHI argument since both incoming edges are not from the
> condition block they won't have EDGE_{TRUE,FALSE}_VALUE set.
Sure thing, I also noticed that in form 4 the both edge of PHI are false, thus
I am working on another patch like extract_true_false_args_from_binary_phi
to take care of this. Let me append that patch to the series v3.
Pan
-----Original Message-----
From: Richard Biener <richard.guent...@gmail.com>
Sent: Monday, September 9, 2024 8:27 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 v2 1/2] Genmatch: Support control flow graph case 1 for phi
on condition
On Thu, Sep 5, 2024 at 2:01 PM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> The gen_phi_on_cond can only support below control flow for cond
> from day 1. Aka:
>
> +------+
> | def |
> | ... | +-----+
> | cond |------>| def |
> +------+ | ... |
> | +-----+
> | |
> v |
> +-----+ |
> | PHI |<----------+
> +-----+
>
> Unfortunately, there will be more scenarios of control flow on PHI.
> For example as below:
>
> T __attribute__((noinline)) \
> sat_s_add_##T##_fmt_3 (T x, T y) \
> { \
> T sum; \
> bool overflow = __builtin_add_overflow (x, y, &sum); \
> return overflow ? x < 0 ? MIN : MAX : sum; \
> }
>
> DEF_SAT_S_ADD_FMT_3(int8_t, uint8_t, INT8_MIN, INT8_MAX)
>
> With expanded RTL like below.
> 3 │
> 4 │ __attribute__((noinline))
> 5 │ int8_t sat_s_add_int8_t_fmt_3 (int8_t x, int8_t y)
> 6 │ {
> 7 │ signed char _1;
> 8 │ signed char _2;
> 9 │ int8_t _3;
> 10 │ __complex__ signed char _6;
> 11 │ _Bool _8;
> 12 │ signed char _9;
> 13 │ signed char _10;
> 14 │ signed char _11;
> 15 │
> 16 │ ;; basic block 2, loop depth 0
> 17 │ ;; pred: ENTRY
> 18 │ _6 = .ADD_OVERFLOW (x_4(D), y_5(D));
> 19 │ _2 = IMAGPART_EXPR <_6>;
> 20 │ if (_2 != 0)
> 21 │ goto <bb 4>; [50.00%]
> 22 │ else
> 23 │ goto <bb 3>; [50.00%]
> 24 │ ;; succ: 4
> 25 │ ;; 3
> 26 │
> 27 │ ;; basic block 3, loop depth 0
> 28 │ ;; pred: 2
> 29 │ _1 = REALPART_EXPR <_6>;
> 30 │ goto <bb 5>; [100.00%]
> 31 │ ;; succ: 5
> 32 │
> 33 │ ;; basic block 4, loop depth 0
> 34 │ ;; pred: 2
> 35 │ _8 = x_4(D) < 0;
> 36 │ _9 = (signed char) _8;
> 37 │ _10 = -_9;
> 38 │ _11 = _10 ^ 127;
> 39 │ ;; succ: 5
> 40 │
> 41 │ ;; basic block 5, loop depth 0
> 42 │ ;; pred: 3
> 43 │ ;; 4
> 44 │ # _3 = PHI <_1(3), _11(4)>
> 45 │ return _3;
> 46 │ ;; succ: EXIT
> 47 │
> 48 │ }
>
> The above code will have below control flow which is not supported by
> the gen_phi_on_cond.
>
> +------+
> | def |
> | ... | +-----+
> | cond |------>| def |
> +------+ | ... |
> | +-----+
> | |
> v |
> +-----+ |
> | def | |
> | ... | |
> +-----+ |
> | |
> | |
> v |
> +-----+ |
> | PHI |<----------+
> +-----+
>
> This patch would like to add support above control flow for the
> gen_phi_on_cond. The generated match code looks like below.
>
> 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)
> {
> ...
>
> After this patch:
> basic_block _b1 = gimple_bb (_a1);
> basic_block _b_cond_1;
> if (gimple_phi_num_args (_a1) == 2
> && (control_flow_graph_case_0_match (_b1, &_b_cond_1)
> || control_flow_graph_case_1_match (_b1, &_b_cond_1)))
> {
> ...
>
> The below testsuites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
Sorry to spoil this again, but can you instead create an interface like
gcond *
match_cond_with_phi (gphi *phi, tree *true_arg, tree *false_arg);
That would from a PHI node match up the controlling condition and
initialize {true,false}_arg with the PHI args that match the conditions
true/false case?
I also think for the diamond case you fail to identify the appropriate
true/false PHI argument since both incoming edges are not from the
condition block they won't have EDGE_{TRUE,FALSE}_VALUE set.
Richard.
> gcc/ChangeLog:
>
> * genmatch.cc (dt_operand::gen_phi_on_cond): Add support control
> flow graph case 1 for gen phi on condition.
> * gimple-match-head.cc (control_flow_graph_case_0_match): Add
> new func impl to match case 0 of cfg.
> (control_flow_graph_case_1_match): Ditto but for case 1.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/genmatch.cc | 37 +++++--------
> gcc/gimple-match-head.cc | 115 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 130 insertions(+), 22 deletions(-)
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index a56bd90cb2c..e0ec1c0e928 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3518,43 +3518,36 @@ 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, "basic_block _b_cond_%d;\n", depth);
>
> - fprintf_indent (f, indent, "if (gimple_phi_num_args (_a%d) == 2)\n",
> depth);
> + fprintf_indent (f, indent, "if (gimple_phi_num_args (_a%d) == 2\n", depth);
>
> - indent += 2;
> - fprintf_indent (f, indent, "{\n");
> indent += 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);
> + " && (control_flow_graph_case_0_match (_b%d, &_b_cond_%d)\n",
> + depth, depth);
> 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);
> + " || control_flow_graph_case_1_match (_b%d, &_b_cond_%d)))\n",
> + depth, depth);
> +
> + indent += 2;
> + fprintf_indent (f, indent, "{\n");
> + indent += 2;
>
> fprintf_indent (f, indent,
> - "gcond *_ct_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_db_%d));\n",
> + "gcond *_cond_%d = safe_dyn_cast <gcond *> (*gsi_last_bb
> (_b_cond_%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)\n", depth);
>
> 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);
> + "tree _cond_rhs_%d = gimple_cond_rhs (_cond_%d);\n", depth, depth);
>
> char opname_0[20];
> char opname_1[20];
> @@ -3562,7 +3555,7 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int
> depth)
> gen_opname (opname_0, 0);
>
> 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);
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index 924d3f1e710..eb43e5c58cb 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -375,3 +375,118 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree
> expr2, bool &wascmp, tree (*va
> return true;
> return false;
> }
> +
> +/*
> + * Return TRUE if the cfg matches the below layout by the given b2 in
> + * the first argument. Or return FALSE.
> + *
> + * If return TRUE, the output argument b_out will be updated to the b0
> + * block as below example.
> + *
> + * If return FALSE, the output argument b_out will be NULL_BLOCK.
> + *
> + * |
> + * |
> + * v
> + * +------+
> + * | b0: |
> + * | def | +-----+
> + * | ... | | b1: |
> + * | cond |------>| def |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | b2: | |
> + * | def |<----------+
> + * +-----+
> + */
> +static inline bool
> +control_flow_graph_case_0_match (basic_block b2, basic_block *b_out)
> +{
> + *b_out = NULL;
> +
> + if (EDGE_COUNT (b2->preds) != 2)
> + return false;
> +
> + basic_block pred_0 = EDGE_PRED (b2, 0)->src;
> + basic_block pred_1 = EDGE_PRED (b2, 1)->src;
> +
> + if (pred_0 == NULL || pred_1 == NULL)
> + return false;
> +
> + if (!(EDGE_COUNT (pred_0->succs) == 2 && EDGE_COUNT (pred_1->succs) == 1)
> + && !(EDGE_COUNT (pred_0->succs) == 1 && EDGE_COUNT (pred_1->succs) ==
> 2))
> + return false;
> +
> + basic_block b0 = EDGE_COUNT (pred_0->succs) == 2 ? pred_0 : pred_1;
> + basic_block b1 = EDGE_COUNT (pred_0->succs) == 1 ? pred_0 : pred_1;
> +
> + if (EDGE_COUNT (b1->preds) != 1 || EDGE_PRED (b1, 0)->src != b0)
> + return false;
> +
> + *b_out = b0;
> + return true;
> +}
> +
> +/*
> + * Return TRUE if the cfg matches the below layout by the given b3 in
> + * the first argument. Or return FALSE.
> + *
> + * If return TRUE, the output argument b_out will be updated to the b0
> + * block as below example.
> + *
> + * If return FALSE, the output argument b_out will be NULL.
> + *
> + * |
> + * |
> + * v
> + * +------+
> + * | b0: |
> + * | ... | +-----+
> + * | cond |------>| b2: |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | b1: | |
> + * | ... | |
> + * +-----+ |
> + * | |
> + * | |
> + * v |
> + * +-----+ |
> + * | b3: |<----------+
> + * | ... |
> + * +-----+
> + */
> +static inline bool
> +control_flow_graph_case_1_match (basic_block b3, basic_block *b_out)
> +{
> + *b_out = NULL;
> +
> + if (EDGE_COUNT (b3->preds) != 2)
> + return false;
> +
> + basic_block b1 = EDGE_PRED (b3, 0)->src;
> + basic_block b2 = EDGE_PRED (b3, 1)->src;
> +
> + if (b1 == NULL || b2 == NULL)
> + return false;
> +
> + if (EDGE_COUNT (b1->succs) != 1
> + || EDGE_COUNT (b1->preds) != 1
> + || EDGE_COUNT (b2->succs) != 1
> + || EDGE_COUNT (b2->preds) != 1)
> + return false;
> +
> + basic_block b0 = EDGE_PRED (b1, 0)->src;
> +
> + if (EDGE_COUNT (b0->succs) != 2 || EDGE_PRED (b2, 0)->src != b0)
> + return false;
> +
> + *b_out = b0;
> + return true;
> +}
> --
> 2.43.0
>
