[v1,1/2] Genmatch: Support new flow for phi on condition
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
From: Pan Li <pan2.li@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 below testsuites 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): Add support for
a new control flow when gen phi on condition.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/genmatch.cc | 85 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 76 insertions(+), 9 deletions(-)
Comments
On Wed, Sep 4, 2024 at 9:25 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@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 below testsuites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
I'm lazy - can you please quote genmatch generated code for the condition for
one case?
Thanks,
Richard.
> gcc/ChangeLog:
>
> * genmatch.cc (dt_operand::gen_phi_on_cond): Add support for
> a new control flow when gen phi on condition.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/genmatch.cc | 85 +++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 76 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index a56bd90cb2c..f538df1be62 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3529,28 +3529,95 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> "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);
> + "gcond *_ct_0_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%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",
> + "gcond *_ct_1_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_%d));\n",
> + depth, depth);
> + fprintf_indent (f, indent,
> + "gcond *_ct_a_%d = _ct_0_%d ? _ct_0_%d : _ct_1_%d;\n",
> + depth, depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _db_%d = _ct_0_%d ? _pb_0_%d : _pb_1_%d;\n",
> + depth, depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _other_db_%d = _ct_0_%d ? _pb_1_%d : _pb_0_%d;\n",
> depth, depth, depth, depth);
>
> 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 _e_00_%d = _pb_0_%d->preds ? EDGE_PRED (_pb_0_%d, 0) : NULL;\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _pb_00_%d = _e_00_%d ? _e_00_%d->src : NULL;\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + "gcond *_ct_b_%d = _pb_00_%d ? "
> + "safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_%d)) : NULL;\n",
> + depth, depth, depth);
> +
> + /* Case 1 flow for PHI.
> + * +------+
> + * | def |
> + * | ... | +-----+
> + * | cond |------>| def |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | PHI |<----------+
> + * +-----+
> + */
> + fprintf_indent (f, indent, "if ((_ct_a_%d"
> " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> fprintf_indent (f, indent,
> - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> + " && 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, " ||\n");
> +
> + /* Case 2 flow for PHI.
> + * +------+
> + * | def |
> + * | ... | +-----+
> + * | cond |------>| def |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | def | |
> + * | ... | |
> + * +-----+ |
> + * | |
> + * | |
> + * v |
> + * +-----+ |
> + * | PHI |<----------+
> + * +-----+
> + */
> + fprintf_indent (f, indent,
> + " (_ct_b_%d && _pb_00_%d && EDGE_COUNT (_pb_0_%d->succs) == 1\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + " && EDGE_COUNT (_pb_0_%d->preds) == 1\n", depth);
> fprintf_indent (f, indent,
> - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> + " && EDGE_COUNT (_other_db_%d->preds) == 1\n", 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 == _pb_00_%d))\n", depth, depth);
>
> indent += 2;
> fprintf_indent (f, indent, "{\n");
> indent += 2;
>
> + fprintf_indent (f, indent,
> + "gcond *_ct_%d = _ct_a_%d ? _ct_a_%d : _ct_b_%d;\n",
> + depth, depth, depth, depth);
> fprintf_indent (f, indent,
> "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> --
> 2.43.0
>
> I'm lazy - can you please quote genmatch generated code for the condition for
> one case?
Sure thing, list the before and after covers all the changes to generated code as blow.
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);
................ switch (TREE_CODE (_p0))
................ {
After this patch:
................ basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
................ basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
................ gcond *_ct_0_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1));
................ gcond *_ct_1_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_1));
................ gcond *_ct_a_1 = _ct_0_1 ? _ct_0_1 : _ct_1_1;
................ basic_block _db_1 = _ct_0_1 ? _pb_0_1 : _pb_1_1;
................ basic_block _other_db_1 = _ct_0_1 ? _pb_1_1 : _pb_0_1;
................ edge _e_00_1 = _pb_0_1->preds ? EDGE_PRED (_pb_0_1, 0) : NULL;
................ basic_block _pb_00_1 = _e_00_1 ? _e_00_1->src : NULL;
................ gcond *_ct_b_1 = _pb_00_1 ? safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_1)) : NULL;
................ if ((_ct_a_1 && EDGE_COUNT (_other_db_1->preds) == 1
................ && EDGE_COUNT (_other_db_1->succs) == 1
................ && EDGE_PRED (_other_db_1, 0)->src == _db_1)
................ ||
................ (_ct_b_1 && _pb_00_1 && EDGE_COUNT (_pb_0_1->succs) == 1
................ && EDGE_COUNT (_pb_0_1->preds) == 1
................ && EDGE_COUNT (_other_db_1->preds) == 1
................ && EDGE_COUNT (_other_db_1->succs) == 1
................ && EDGE_PRED (_other_db_1, 0)->src == _pb_00_1))
................ {
................ gcond *_ct_1 = _ct_a_1 ? _ct_a_1 : _ct_b_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);
Pan
-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com>
Sent: Wednesday, September 4, 2024 3:42 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 v1 1/2] Genmatch: Support new flow for phi on condition
On Wed, Sep 4, 2024 at 9:25 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@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 below testsuites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
I'm lazy - can you please quote genmatch generated code for the condition for
one case?
Thanks,
Richard.
> gcc/ChangeLog:
>
> * genmatch.cc (dt_operand::gen_phi_on_cond): Add support for
> a new control flow when gen phi on condition.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/genmatch.cc | 85 +++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 76 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index a56bd90cb2c..f538df1be62 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3529,28 +3529,95 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> "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);
> + "gcond *_ct_0_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%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",
> + "gcond *_ct_1_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_%d));\n",
> + depth, depth);
> + fprintf_indent (f, indent,
> + "gcond *_ct_a_%d = _ct_0_%d ? _ct_0_%d : _ct_1_%d;\n",
> + depth, depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _db_%d = _ct_0_%d ? _pb_0_%d : _pb_1_%d;\n",
> + depth, depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _other_db_%d = _ct_0_%d ? _pb_1_%d : _pb_0_%d;\n",
> depth, depth, depth, depth);
>
> 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 _e_00_%d = _pb_0_%d->preds ? EDGE_PRED (_pb_0_%d, 0) : NULL;\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + "basic_block _pb_00_%d = _e_00_%d ? _e_00_%d->src : NULL;\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + "gcond *_ct_b_%d = _pb_00_%d ? "
> + "safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_%d)) : NULL;\n",
> + depth, depth, depth);
> +
> + /* Case 1 flow for PHI.
> + * +------+
> + * | def |
> + * | ... | +-----+
> + * | cond |------>| def |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | PHI |<----------+
> + * +-----+
> + */
> + fprintf_indent (f, indent, "if ((_ct_a_%d"
> " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> fprintf_indent (f, indent,
> - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> + " && 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, " ||\n");
> +
> + /* Case 2 flow for PHI.
> + * +------+
> + * | def |
> + * | ... | +-----+
> + * | cond |------>| def |
> + * +------+ | ... |
> + * | +-----+
> + * | |
> + * v |
> + * +-----+ |
> + * | def | |
> + * | ... | |
> + * +-----+ |
> + * | |
> + * | |
> + * v |
> + * +-----+ |
> + * | PHI |<----------+
> + * +-----+
> + */
> + fprintf_indent (f, indent,
> + " (_ct_b_%d && _pb_00_%d && EDGE_COUNT (_pb_0_%d->succs) == 1\n",
> + depth, depth, depth);
> + fprintf_indent (f, indent,
> + " && EDGE_COUNT (_pb_0_%d->preds) == 1\n", depth);
> fprintf_indent (f, indent,
> - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> + " && EDGE_COUNT (_other_db_%d->preds) == 1\n", 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 == _pb_00_%d))\n", depth, depth);
>
> indent += 2;
> fprintf_indent (f, indent, "{\n");
> indent += 2;
>
> + fprintf_indent (f, indent,
> + "gcond *_ct_%d = _ct_a_%d ? _ct_a_%d : _ct_b_%d;\n",
> + depth, depth, depth, depth);
> fprintf_indent (f, indent,
> "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> fprintf_indent (f, indent,
> --
> 2.43.0
>
On Wed, Sep 4, 2024 at 9:48 AM Li, Pan2 <pan2.li@intel.com> wrote:
>
> > I'm lazy - can you please quote genmatch generated code for the condition for
> > one case?
>
> Sure thing, list the before and after covers all the changes to generated code as blow.
>
> 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);
> ................ switch (TREE_CODE (_p0))
> ................ {
>
> After this patch:
> ................ basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
> ................ basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
> ................ gcond *_ct_0_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1));
> ................ gcond *_ct_1_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_1));
> ................ gcond *_ct_a_1 = _ct_0_1 ? _ct_0_1 : _ct_1_1;
> ................ basic_block _db_1 = _ct_0_1 ? _pb_0_1 : _pb_1_1;
> ................ basic_block _other_db_1 = _ct_0_1 ? _pb_1_1 : _pb_0_1;
> ................ edge _e_00_1 = _pb_0_1->preds ? EDGE_PRED (_pb_0_1, 0) : NULL;
> ................ basic_block _pb_00_1 = _e_00_1 ? _e_00_1->src : NULL;
> ................ gcond *_ct_b_1 = _pb_00_1 ? safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_1)) : NULL;
> ................ if ((_ct_a_1 && EDGE_COUNT (_other_db_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->succs) == 1
> ................ && EDGE_PRED (_other_db_1, 0)->src == _db_1)
> ................ ||
> ................ (_ct_b_1 && _pb_00_1 && EDGE_COUNT (_pb_0_1->succs) == 1
> ................ && EDGE_COUNT (_pb_0_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->succs) == 1
> ................ && EDGE_PRED (_other_db_1, 0)->src == _pb_00_1))
> ................ {
> ................ gcond *_ct_1 = _ct_a_1 ? _ct_a_1 : _ct_b_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);
I think it might be better to refactor this to detect the three CFGs like
if (EDGE_COUNT (_pb_0_1->preds) == 1
&& EDGE_PRED (_pb_0_1, 0)->src == pb_1_1)
{
.. check rest of constraints ..
}
else if (... same for _pb_1_1 being the forwarder ...)
...
else if (EDGE_COUNT (_pb_0_1->preds) == 1
&& EDGE_COUNT (_pb_1_1->preds) == 1
&& EDGE_PRED (_pb_0_1, 0)->src == EDGE_PRED (_pb_1_1, 0)->src)
...
I also think we may want to split out this CFG matching code out into
a helper function
in gimple-match-head.cc instead of repeating it fully for each pattern?
Thanks,
Richard.
> Pan
>
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Wednesday, September 4, 2024 3:42 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 v1 1/2] Genmatch: Support new flow for phi on condition
>
> On Wed, Sep 4, 2024 at 9:25 AM <pan2.li@intel.com> wrote:
> >
> > From: Pan Li <pan2.li@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 below testsuites are passed for this patch:
> > * The rv64gcv fully regression test.
> > * The x86 bootstrap test.
> > * The x86 fully regression test.
>
> I'm lazy - can you please quote genmatch generated code for the condition for
> one case?
>
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > * genmatch.cc (dt_operand::gen_phi_on_cond): Add support for
> > a new control flow when gen phi on condition.
> >
> > Signed-off-by: Pan Li <pan2.li@intel.com>
> > ---
> > gcc/genmatch.cc | 85 +++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 76 insertions(+), 9 deletions(-)
> >
> > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> > index a56bd90cb2c..f538df1be62 100644
> > --- a/gcc/genmatch.cc
> > +++ b/gcc/genmatch.cc
> > @@ -3529,28 +3529,95 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> > "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);
> > + "gcond *_ct_0_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%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",
> > + "gcond *_ct_1_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_%d));\n",
> > + depth, depth);
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_a_%d = _ct_0_%d ? _ct_0_%d : _ct_1_%d;\n",
> > + depth, depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _db_%d = _ct_0_%d ? _pb_0_%d : _pb_1_%d;\n",
> > + depth, depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _other_db_%d = _ct_0_%d ? _pb_1_%d : _pb_0_%d;\n",
> > depth, depth, depth, depth);
> >
> > 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 _e_00_%d = _pb_0_%d->preds ? EDGE_PRED (_pb_0_%d, 0) : NULL;\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _pb_00_%d = _e_00_%d ? _e_00_%d->src : NULL;\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_b_%d = _pb_00_%d ? "
> > + "safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_%d)) : NULL;\n",
> > + depth, depth, depth);
> > +
> > + /* Case 1 flow for PHI.
> > + * +------+
> > + * | def |
> > + * | ... | +-----+
> > + * | cond |------>| def |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | PHI |<----------+
> > + * +-----+
> > + */
> > + fprintf_indent (f, indent, "if ((_ct_a_%d"
> > " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> > fprintf_indent (f, indent,
> > - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> > + " && 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, " ||\n");
> > +
> > + /* Case 2 flow for PHI.
> > + * +------+
> > + * | def |
> > + * | ... | +-----+
> > + * | cond |------>| def |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | def | |
> > + * | ... | |
> > + * +-----+ |
> > + * | |
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | PHI |<----------+
> > + * +-----+
> > + */
> > + fprintf_indent (f, indent,
> > + " (_ct_b_%d && _pb_00_%d && EDGE_COUNT (_pb_0_%d->succs) == 1\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + " && EDGE_COUNT (_pb_0_%d->preds) == 1\n", depth);
> > fprintf_indent (f, indent,
> > - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> > + " && EDGE_COUNT (_other_db_%d->preds) == 1\n", 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 == _pb_00_%d))\n", depth, depth);
> >
> > indent += 2;
> > fprintf_indent (f, indent, "{\n");
> > indent += 2;
> >
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_%d = _ct_a_%d ? _ct_a_%d : _ct_b_%d;\n",
> > + depth, depth, depth, depth);
> > fprintf_indent (f, indent,
> > "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> > fprintf_indent (f, indent,
> > --
> > 2.43.0
> >
Thanks Richard for comments.
> I also think we may want to split out this CFG matching code out into
> a helper function
> in gimple-match-head.cc instead of repeating it fully for each pattern?
That makes sense to me, let me have a try in v2.
Pan
-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com>
Sent: Wednesday, September 4, 2024 6:56 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 v1 1/2] Genmatch: Support new flow for phi on condition
On Wed, Sep 4, 2024 at 9:48 AM Li, Pan2 <pan2.li@intel.com> wrote:
>
> > I'm lazy - can you please quote genmatch generated code for the condition for
> > one case?
>
> Sure thing, list the before and after covers all the changes to generated code as blow.
>
> 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);
> ................ switch (TREE_CODE (_p0))
> ................ {
>
> After this patch:
> ................ basic_block _pb_0_1 = EDGE_PRED (_b1, 0)->src;
> ................ basic_block _pb_1_1 = EDGE_PRED (_b1, 1)->src;
> ................ gcond *_ct_0_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_1));
> ................ gcond *_ct_1_1 = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_1));
> ................ gcond *_ct_a_1 = _ct_0_1 ? _ct_0_1 : _ct_1_1;
> ................ basic_block _db_1 = _ct_0_1 ? _pb_0_1 : _pb_1_1;
> ................ basic_block _other_db_1 = _ct_0_1 ? _pb_1_1 : _pb_0_1;
> ................ edge _e_00_1 = _pb_0_1->preds ? EDGE_PRED (_pb_0_1, 0) : NULL;
> ................ basic_block _pb_00_1 = _e_00_1 ? _e_00_1->src : NULL;
> ................ gcond *_ct_b_1 = _pb_00_1 ? safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_1)) : NULL;
> ................ if ((_ct_a_1 && EDGE_COUNT (_other_db_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->succs) == 1
> ................ && EDGE_PRED (_other_db_1, 0)->src == _db_1)
> ................ ||
> ................ (_ct_b_1 && _pb_00_1 && EDGE_COUNT (_pb_0_1->succs) == 1
> ................ && EDGE_COUNT (_pb_0_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->preds) == 1
> ................ && EDGE_COUNT (_other_db_1->succs) == 1
> ................ && EDGE_PRED (_other_db_1, 0)->src == _pb_00_1))
> ................ {
> ................ gcond *_ct_1 = _ct_a_1 ? _ct_a_1 : _ct_b_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);
I think it might be better to refactor this to detect the three CFGs like
if (EDGE_COUNT (_pb_0_1->preds) == 1
&& EDGE_PRED (_pb_0_1, 0)->src == pb_1_1)
{
.. check rest of constraints ..
}
else if (... same for _pb_1_1 being the forwarder ...)
...
else if (EDGE_COUNT (_pb_0_1->preds) == 1
&& EDGE_COUNT (_pb_1_1->preds) == 1
&& EDGE_PRED (_pb_0_1, 0)->src == EDGE_PRED (_pb_1_1, 0)->src)
...
I also think we may want to split out this CFG matching code out into
a helper function
in gimple-match-head.cc instead of repeating it fully for each pattern?
Thanks,
Richard.
> Pan
>
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Wednesday, September 4, 2024 3:42 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 v1 1/2] Genmatch: Support new flow for phi on condition
>
> On Wed, Sep 4, 2024 at 9:25 AM <pan2.li@intel.com> wrote:
> >
> > From: Pan Li <pan2.li@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 below testsuites are passed for this patch:
> > * The rv64gcv fully regression test.
> > * The x86 bootstrap test.
> > * The x86 fully regression test.
>
> I'm lazy - can you please quote genmatch generated code for the condition for
> one case?
>
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > * genmatch.cc (dt_operand::gen_phi_on_cond): Add support for
> > a new control flow when gen phi on condition.
> >
> > Signed-off-by: Pan Li <pan2.li@intel.com>
> > ---
> > gcc/genmatch.cc | 85 +++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 76 insertions(+), 9 deletions(-)
> >
> > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> > index a56bd90cb2c..f538df1be62 100644
> > --- a/gcc/genmatch.cc
> > +++ b/gcc/genmatch.cc
> > @@ -3529,28 +3529,95 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
> > "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);
> > + "gcond *_ct_0_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%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",
> > + "gcond *_ct_1_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_%d));\n",
> > + depth, depth);
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_a_%d = _ct_0_%d ? _ct_0_%d : _ct_1_%d;\n",
> > + depth, depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _db_%d = _ct_0_%d ? _pb_0_%d : _pb_1_%d;\n",
> > + depth, depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _other_db_%d = _ct_0_%d ? _pb_1_%d : _pb_0_%d;\n",
> > depth, depth, depth, depth);
> >
> > 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 _e_00_%d = _pb_0_%d->preds ? EDGE_PRED (_pb_0_%d, 0) : NULL;\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "basic_block _pb_00_%d = _e_00_%d ? _e_00_%d->src : NULL;\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_b_%d = _pb_00_%d ? "
> > + "safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_%d)) : NULL;\n",
> > + depth, depth, depth);
> > +
> > + /* Case 1 flow for PHI.
> > + * +------+
> > + * | def |
> > + * | ... | +-----+
> > + * | cond |------>| def |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | PHI |<----------+
> > + * +-----+
> > + */
> > + fprintf_indent (f, indent, "if ((_ct_a_%d"
> > " && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
> > fprintf_indent (f, indent,
> > - " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
> > + " && 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, " ||\n");
> > +
> > + /* Case 2 flow for PHI.
> > + * +------+
> > + * | def |
> > + * | ... | +-----+
> > + * | cond |------>| def |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | def | |
> > + * | ... | |
> > + * +-----+ |
> > + * | |
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | PHI |<----------+
> > + * +-----+
> > + */
> > + fprintf_indent (f, indent,
> > + " (_ct_b_%d && _pb_00_%d && EDGE_COUNT (_pb_0_%d->succs) == 1\n",
> > + depth, depth, depth);
> > + fprintf_indent (f, indent,
> > + " && EDGE_COUNT (_pb_0_%d->preds) == 1\n", depth);
> > fprintf_indent (f, indent,
> > - " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
> > + " && EDGE_COUNT (_other_db_%d->preds) == 1\n", 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 == _pb_00_%d))\n", depth, depth);
> >
> > indent += 2;
> > fprintf_indent (f, indent, "{\n");
> > indent += 2;
> >
> > + fprintf_indent (f, indent,
> > + "gcond *_ct_%d = _ct_a_%d ? _ct_a_%d : _ct_b_%d;\n",
> > + depth, depth, depth, depth);
> > fprintf_indent (f, indent,
> > "tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
> > fprintf_indent (f, indent,
> > --
> > 2.43.0
> >
@@ -3529,28 +3529,95 @@ dt_operand::gen_phi_on_cond (FILE *f, int indent, int depth)
"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);
+ "gcond *_ct_0_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_0_%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",
+ "gcond *_ct_1_%d = safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_1_%d));\n",
+ depth, depth);
+ fprintf_indent (f, indent,
+ "gcond *_ct_a_%d = _ct_0_%d ? _ct_0_%d : _ct_1_%d;\n",
+ depth, depth, depth, depth);
+ fprintf_indent (f, indent,
+ "basic_block _db_%d = _ct_0_%d ? _pb_0_%d : _pb_1_%d;\n",
+ depth, depth, depth, depth);
+ fprintf_indent (f, indent,
+ "basic_block _other_db_%d = _ct_0_%d ? _pb_1_%d : _pb_0_%d;\n",
depth, depth, depth, depth);
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 _e_00_%d = _pb_0_%d->preds ? EDGE_PRED (_pb_0_%d, 0) : NULL;\n",
+ depth, depth, depth);
+ fprintf_indent (f, indent,
+ "basic_block _pb_00_%d = _e_00_%d ? _e_00_%d->src : NULL;\n",
+ depth, depth, depth);
+ fprintf_indent (f, indent,
+ "gcond *_ct_b_%d = _pb_00_%d ? "
+ "safe_dyn_cast <gcond *> (*gsi_last_bb (_pb_00_%d)) : NULL;\n",
+ depth, depth, depth);
+
+ /* Case 1 flow for PHI.
+ * +------+
+ * | def |
+ * | ... | +-----+
+ * | cond |------>| def |
+ * +------+ | ... |
+ * | +-----+
+ * | |
+ * v |
+ * +-----+ |
+ * | PHI |<----------+
+ * +-----+
+ */
+ fprintf_indent (f, indent, "if ((_ct_a_%d"
" && EDGE_COUNT (_other_db_%d->preds) == 1\n", depth, depth);
fprintf_indent (f, indent,
- " && EDGE_COUNT (_other_db_%d->succs) == 1\n", depth);
+ " && 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, " ||\n");
+
+ /* Case 2 flow for PHI.
+ * +------+
+ * | def |
+ * | ... | +-----+
+ * | cond |------>| def |
+ * +------+ | ... |
+ * | +-----+
+ * | |
+ * v |
+ * +-----+ |
+ * | def | |
+ * | ... | |
+ * +-----+ |
+ * | |
+ * | |
+ * v |
+ * +-----+ |
+ * | PHI |<----------+
+ * +-----+
+ */
+ fprintf_indent (f, indent,
+ " (_ct_b_%d && _pb_00_%d && EDGE_COUNT (_pb_0_%d->succs) == 1\n",
+ depth, depth, depth);
+ fprintf_indent (f, indent,
+ " && EDGE_COUNT (_pb_0_%d->preds) == 1\n", depth);
fprintf_indent (f, indent,
- " && EDGE_PRED (_other_db_%d, 0)->src == _db_%d)\n", depth, depth);
+ " && EDGE_COUNT (_other_db_%d->preds) == 1\n", 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 == _pb_00_%d))\n", depth, depth);
indent += 2;
fprintf_indent (f, indent, "{\n");
indent += 2;
+ fprintf_indent (f, indent,
+ "gcond *_ct_%d = _ct_a_%d ? _ct_a_%d : _ct_b_%d;\n",
+ depth, depth, depth, depth);
fprintf_indent (f, indent,
"tree _cond_lhs_%d = gimple_cond_lhs (_ct_%d);\n", depth, depth);
fprintf_indent (f, indent,