Got, thanks Richard and will have a try in v5.
Pan
-----Original Message-----
From: Richard Biener <richard.guent...@gmail.com>
Sent: Wednesday, September 18, 2024 8:06 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 v4 1/4] Match: Add interface match_cond_with_binary_phi for
true/false arg
On Wed, Sep 18, 2024 at 2:02 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Fri, Sep 13, 2024 at 12:42 AM <pan2...@intel.com> wrote:
> >
> > From: Pan Li <pan2...@intel.com>
> >
> > When matching the cond with 2 args phi node, we need to figure out
> > which arg of phi node comes from the true edge of cond block, as
> > well as the false edge. This patch would like to add interface
> > to perform the action and return the true and false arg in TREE type.
> >
> > There will be some additional handling if one of the arg is INTEGER_CST.
> > Because the INTEGER_CST args may have no source block, thus its' edge
> > source points to the condition block. See below example in line 31,
> > the 255 INTEGER_CST has block 2 as source. Thus, we need to find
> > the non-INTEGER_CST (aka _1) to tell which one is the true/false edge.
> > For example, the _1(3) takes block 3 as source, which is the dest
> > of false edge of the condition block.
> >
> > 4 │ __attribute__((noinline))
> > 5 │ uint8_t sat_u_add_imm_type_check_uint8_t_fmt_2 (uint8_t x)
> > 6 │ {
> > 7 │ unsigned char _1;
> > 8 │ unsigned char _2;
> > 9 │ uint8_t _3;
> > 10 │ __complex__ unsigned char _5;
> > 11 │
> > 12 │ ;; basic block 2, loop depth 0
> > 13 │ ;; pred: ENTRY
> > 14 │ _5 = .ADD_OVERFLOW (x_4(D), 9);
> > 15 │ _2 = IMAGPART_EXPR <_5>;
> > 16 │ if (_2 != 0)
> > 17 │ goto <bb 4>; [35.00%]
> > 18 │ else
> > 19 │ goto <bb 3>; [65.00%]
> > 20 │ ;; succ: 3
> > 21 │ ;; 4
> > 22 │
> > 23 │ ;; basic block 3, loop depth 0
> > 24 │ ;; pred: 2
> > 25 │ _1 = REALPART_EXPR <_5>;
> > 26 │ ;; succ: 4
> > 27 │
> > 28 │ ;; basic block 4, loop depth 0
> > 29 │ ;; pred: 2
> > 30 │ ;; 3
> > 31 │ # _3 = PHI <255(2), _1(3)>
> > 32 │ return _3;
> > 33 │ ;; succ: EXIT
> > 34 │
> > 35 │ }
> >
> > 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:
> >
> > * gimple-match-head.cc (match_cond_with_binary_phi): Add new func
> > impl to match binary phi for true and false arg.
> >
> > Signed-off-by: Pan Li <pan2...@intel.com>
> > ---
> > gcc/gimple-match-head.cc | 118 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 118 insertions(+)
> >
> > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> > index 924d3f1e710..6e7a3a0d62e 100644
> > --- a/gcc/gimple-match-head.cc
> > +++ b/gcc/gimple-match-head.cc
> > @@ -375,3 +375,121 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree
> > expr2, bool &wascmp, tree (*va
> > return true;
> > return false;
> > }
> > +
> > +/*
> > + * Return the relevant gcond * of the given phi, as well as the true
> > + * and false TREE args of the phi. Or return NULL.
> > + *
> > + * If matched the gcond *, the output argument TREE true_arg and false_arg
> > + * will be updated to the relevant args of phi.
> > + *
> > + * If failed to match, NULL gcond * will be returned, as well as the output
> > + * arguments will be set to NULL_TREE.
> > + */
> > +
> > +static inline gcond *
> > +match_cond_with_binary_phi (gphi *phi, tree *true_arg, tree *false_arg)
> > +{
> > + *true_arg = *false_arg = NULL_TREE;
> > +
> > + if (gimple_phi_num_args (phi) != 2
> > + || EDGE_COUNT (gimple_bb (phi)->preds) != 2)
> > + return NULL;
> > +
> > + basic_block pred_0 = EDGE_PRED (gimple_bb (phi), 0)->src;
> > + basic_block pred_1 = EDGE_PRED (gimple_bb (phi), 1)->src;
> > + basic_block cond_block = NULL;
> > +
> > + 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))
> > + {
> > + /* For below control flow graph:
> > + * |
> > + * v
> > + * +------+
> > + * | b0: |
> > + * | def | +-----+
> > + * | ... | | b1: |
> > + * | cond |------>| def |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | b2: | |
> > + * | def |<----------+
> > + * +-----+
> > + */
> > + 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)
> > + cond_block = b0;
> > + }
> > +
> > + if (EDGE_COUNT (pred_0->succs) == 1 && EDGE_COUNT (pred_0->preds) == 1
> > + && EDGE_COUNT (pred_1->succs) == 1 && EDGE_COUNT (pred_1->preds) ==
> > 1)
> > + {
> > + /* For below control flow graph:
> > + * |
> > + * v
> > + * +------+
> > + * | b0: |
> > + * | ... | +-----+
> > + * | cond |------>| b2: |
> > + * +------+ | ... |
> > + * | +-----+
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | b1: | |
> > + * | ... | |
> > + * +-----+ |
> > + * | |
> > + * | |
> > + * v |
> > + * +-----+ |
> > + * | b3: |<----------+
> > + * | ... |
> > + * +-----+
> > + */
> > + basic_block b0 = EDGE_PRED (pred_0, 0)->src;
> > +
> > + if (EDGE_COUNT (b0->succs) == 2 && EDGE_PRED (pred_1, 0)->src == b0)
> > + cond_block = b0;
> > + }
> > +
> > + if (cond_block == NULL)
> > + return NULL;
>
> I think it's better to above compute the edge from the possible gcond block
> that
> leads to PHI arg0, because the CFG matching easily identifies it and we
> can take the edge flag from it. Like maybe (untested):
>
> static inline gcond *
> match_cond_with_binary_phi (gphi *phi, tree *true_arg, tree *false_arg)
> {
> *true_arg = *false_arg = NULL_TREE;
>
> if (gimple_phi_num_args (phi) != 2)
> return NULL;
>
> basic_block pred0 = EDGE_PRED (gimple_bb (phi), 0)->src;
> basic_block pred1 = EDGE_PRED (gimple_bb (phi), 1)->src;
> gcond *cond;
> edge edge_for_pred0;
>
> if (EDGE_COUNT (pred0->succs) == 2
> && EDGE_COUNT (pred1->succs) == 1
> && EDGE_COUNT (pred1->preds) == 1
> && pred0->src == EDGE_PRED (pred1->preds, 0)->src)
> edge_for_pred0 = EDGE_PRED (gimple_bb (phi), 0);
>
> else if (EDGE_COUNT (pred1->succs) == 2
> && EDGE_COUNT (pred0->succs) == 1
> && EDGE_COUNT (pred0->preds) == 1
> && EDGE_PRED (pred0->preds, 0)->src == pred1->src)
> edge_for_pred0 = EDGE_PRED (pred0, 0);
>
> else if (EDGE_COUNT (pred0->succs) == 1
> && EDGE_COUNT (pred1->succs) == 1
> && EDGE_COUNT (pred0->preds) == 1
> && EDGE_COUNT (pred1->preds) == 1
> && EDGE_COUNT (EDGE_SUCC (EDGE_PRED (pred0, 0)->src)) == 2
> && EDGE_PRED (pred0, 0)->src == EDGE_PRED (pred1, 0)->src)
> edge_for_pred0 = EDGE_PRED (pred0, 0);
>
> gcond *cond = safe_dyn_cast <gcond *> (*gsi_last_bb (edge_for_pred0->src));
> if (!cond)
> return nullptr;
>
> if (edge_for_pred0->flags & EDGE_TRUE_VALUE)
> {
> *true_arg = gimple_phi_arg_def (phi, 0);
> *false_arg = gimple_phi_arg_def (phi, 1);
> }
> else /* edge_for_pred0->flags & EDGE_FALSE_VALUE */
> {
> *true_arg = gimple_phi_arg_def (phi, 0);
> *false_arg = gimple_phi_arg_def (phi, 1);
> }
>
> return cond;
> }
Sorry, that doesn't evne build, the following does:
static inline gcond *
match_cond_with_binary_phi (gphi *phi, tree *true_arg, tree *false_arg)
{
*true_arg = *false_arg = NULL_TREE;
if (gimple_phi_num_args (phi) != 2)
return NULL;
basic_block pred0 = EDGE_PRED (gimple_bb (phi), 0)->src;
basic_block pred1 = EDGE_PRED (gimple_bb (phi), 1)->src;
edge edge_for_pred0;
if (EDGE_COUNT (pred0->succs) == 2
&& EDGE_COUNT (pred1->succs) == 1
&& EDGE_COUNT (pred1->preds) == 1
&& pred0 == EDGE_PRED (pred1, 0)->src)
edge_for_pred0 = EDGE_PRED (gimple_bb (phi), 0);
else if (EDGE_COUNT (pred1->succs) == 2
&& EDGE_COUNT (pred0->succs) == 1
&& EDGE_COUNT (pred0->preds) == 1
&& EDGE_PRED (pred0, 0)->src == pred1)
edge_for_pred0 = EDGE_PRED (pred0, 0);
else if (EDGE_COUNT (pred0->succs) == 1
&& EDGE_COUNT (pred1->succs) == 1
&& EDGE_COUNT (pred0->preds) == 1
&& EDGE_COUNT (pred1->preds) == 1
&& EDGE_COUNT (EDGE_PRED (pred0, 0)->src->succs) == 2
&& EDGE_PRED (pred0, 0)->src == EDGE_PRED (pred1, 0)->src)
edge_for_pred0 = EDGE_PRED (pred0, 0);
gcond *cond = safe_dyn_cast <gcond *> (*gsi_last_bb (edge_for_pred0->src));
if (!cond)
return nullptr;
if (edge_for_pred0->flags & EDGE_TRUE_VALUE)
{
*true_arg = gimple_phi_arg_def (phi, 0);
*false_arg = gimple_phi_arg_def (phi, 1);
}
else /* edge_for_pred0->flags & EDGE_FALSE_VALUE */
{
*true_arg = gimple_phi_arg_def (phi, 0);
*false_arg = gimple_phi_arg_def (phi, 1);
}
return cond;
}
>
> > + /* Now we locate the cond_block for phi node. */
> > + tree t0 = gimple_phi_arg_def (phi, 0);
> > + tree t1 = gimple_phi_arg_def (phi, 1);
> > + edge e0 = gimple_phi_arg_edge (phi, 0);
> > + edge e1 = gimple_phi_arg_edge (phi, 1);
> > +
> > + if (TREE_CODE (t0) == INTEGER_CST && TREE_CODE (t1) == INTEGER_CST)
> > + return NULL;
> > +
> > + bool arg_0_cst_p = TREE_CODE (t0) == INTEGER_CST;
> > + edge arg_edge = arg_0_cst_p ? e1 : e0;
> > + tree arg = arg_0_cst_p ? t1 : t0;
> > + tree other_arg = arg_0_cst_p ? t0 : t1;
> > +
> > + edge cond_e0 = EDGE_SUCC (cond_block, 0);
> > + edge cond_e1 = EDGE_SUCC (cond_block, 1);
> > + edge matched_edge = arg_edge->src == cond_e0->dest ? cond_e0 : cond_e1;
> > +
> > + if (matched_edge->flags & EDGE_TRUE_VALUE)
> > + {
> > + *true_arg = arg;
> > + *false_arg = other_arg;
> > + }
> > + else
> > + {
> > + *false_arg = arg;
> > + *true_arg = other_arg;
> > + }
> > +
> > + return safe_dyn_cast <gcond *> (*gsi_last_bb (cond_block));
> > +}
> > --
> > 2.43.0
> >
