tcg/tcg.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-)
This patch extends reachable_code_pass() to also deal with empty
else-branches of the form
br $L0
set_label $L1
set_label $L0
converting them to
set_label $L1
when $L0 is only referenced by the br op. This type of empty-else
branch will be emitted by idef-parser in the Hexagon frontend once
CANCEL statements have been ignored.
Signed-off-by: Anton Johansson <anjo@rev.ng>
---
tcg/tcg.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index a4a3da6804..531bc74231 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2664,21 +2664,40 @@ static void reachable_code_pass(TCGContext *s)
dead = false;
remove = false;
- /*
- * Optimization can fold conditional branches to unconditional.
- * If we find a label with one reference which is preceded by
- * an unconditional branch to it, remove both. This needed to
- * wait until the dead code in between them was removed.
- */
- if (label->refs == 1) {
- TCGOp *op_prev = QTAILQ_PREV(op, link);
- if (op_prev->opc == INDEX_op_br &&
- label == arg_label(op_prev->args[0])) {
+ TCGOp *op_prev = QTAILQ_PREV(op, link);
+ if (label->refs == 1 &&
+ op_prev->opc == INDEX_op_br &&
+ label == arg_label(op_prev->args[0])) {
+ /*
+ * Optimization can fold conditional branches to
+ * unconditional. If we find a label with one reference
+ * which is preceded by an unconditional branch to it,
+ * remove both. This needed to wait until the dead code
+ * in between them was removed.
+ */
+ tcg_op_remove(s, op_prev);
+ remove = true;
+ } else if (op_next->opc == INDEX_op_set_label) {
+ /*
+ * The Hexagon frontend can generate empty else-branches for
+ * certain instructions. If we encounter
+ *
+ * br $L0
+ * set_label $L1
+ * set_label $L0
+ *
+ * where $L0 only has a single reference, remove the branch
+ * to $L0 and the corresonding label.
+ */
+ TCGLabel *next_label = arg_label(op_next->args[0]);
+ if (next_label->refs == 1 &&
+ op_prev->opc == INDEX_op_br &&
+ next_label == arg_label(op_prev->args[0])) {
tcg_op_remove(s, op_prev);
- remove = true;
}
}
}
+
break;
case INDEX_op_br:
--
2.39.1
> -----Original Message----- > From: Anton Johansson <anjo@rev.ng> > Sent: Wednesday, March 1, 2023 7:22 AM > To: qemu-devel@nongnu.org > Cc: ale@rev.ng; richard.henderson@linaro.org; Taylor Simpson > <tsimpson@quicinc.com> > Subject: [PATCH] tcg: `reachable_code_pass()` remove empty else-branch > > This patch extends reachable_code_pass() to also deal with empty else- > branches of the form > > br $L0 > set_label $L1 > set_label $L0 > > converting them to > > set_label $L1 > > when $L0 is only referenced by the br op. This type of empty-else branch will > be emitted by idef-parser in the Hexagon frontend once CANCEL statements > have been ignored. > > Signed-off-by: Anton Johansson <anjo@rev.ng> > --- > tcg/tcg.c | 41 ++++++++++++++++++++++++++++++----------- > 1 file changed, 30 insertions(+), 11 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index a4a3da6804..531bc74231 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -2664,21 +2664,40 @@ static void reachable_code_pass(TCGContext *s) > dead = false; > remove = false; > > - /* > - * Optimization can fold conditional branches to unconditional. > - * If we find a label with one reference which is preceded by > - * an unconditional branch to it, remove both. This needed to > - * wait until the dead code in between them was removed. > - */ > - if (label->refs == 1) { > - TCGOp *op_prev = QTAILQ_PREV(op, link); Can't we just insert a while loop here to move op_prev back across labels? while (op_next->opc == INDEX_op_set_label) { op_prev = QTAILQ_PREV(op, op_prev); } > - if (op_prev->opc == INDEX_op_br && > - label == arg_label(op_prev->args[0])) { Also, here is the patch that exposes the need for this optimization https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg07236.html Thanks, Taylor
On 3/1/23 23:37, Taylor Simpson wrote: >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index a4a3da6804..531bc74231 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -2664,21 +2664,40 @@ static void reachable_code_pass(TCGContext *s) >> dead = false; >> remove = false; >> >> - /* >> - * Optimization can fold conditional branches to unconditional. >> - * If we find a label with one reference which is preceded by >> - * an unconditional branch to it, remove both. This needed to >> - * wait until the dead code in between them was removed. >> - */ >> - if (label->refs == 1) { >> - TCGOp *op_prev = QTAILQ_PREV(op, link); > Can't we just insert a while loop here to move op_prev back across labels? > > while (op_next->opc == INDEX_op_set_label) { > op_prev = QTAILQ_PREV(op, op_prev); > } > >> - if (op_prev->opc == INDEX_op_br && >> - label == arg_label(op_prev->args[0])) { Ah I see, you're thinking something like dead = false; remove = false; if (label->refs == 1) { TCGOp *op_prev = NULL; do { op_prev = QTAILQ_PREV(op_prev, link); } while (op_prev->opc == INDEX_op_set_label); if (op_prev->opc == INDEX_op_br && label == arg_label(op_prev->args[0])) { tcg_op_remove(s, op_prev); remove = true; } } to handle br set_label ... set_label I do like being able to combine both branches, and we're no longer relying on the next iteration of the loop to clean up the final set_label. Since we won't ever encounter more than one intermediate set_label this might be overkill, but either way I think it's an improvement. Thanks for the feedback, I'll resubmit with this change! :) -- Anton Johansson, rev.ng Labs Srl.
> -----Original Message----- > From: Anton Johansson <anjo@rev.ng> > Sent: Friday, March 3, 2023 7:32 AM > To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org > Cc: ale@rev.ng; richard.henderson@linaro.org > Subject: Re: [PATCH] tcg: `reachable_code_pass()` remove empty else- > branch > > On 3/1/23 23:37, Taylor Simpson wrote: > >> diff --git a/tcg/tcg.c b/tcg/tcg.c > >> index a4a3da6804..531bc74231 100644 > >> --- a/tcg/tcg.c > >> +++ b/tcg/tcg.c > >> @@ -2664,21 +2664,40 @@ static void reachable_code_pass(TCGContext > *s) > >> dead = false; > >> remove = false; > >> > >> - /* > >> - * Optimization can fold conditional branches to unconditional. > >> - * If we find a label with one reference which is preceded by > >> - * an unconditional branch to it, remove both. This needed to > >> - * wait until the dead code in between them was removed. > >> - */ > >> - if (label->refs == 1) { > >> - TCGOp *op_prev = QTAILQ_PREV(op, link); > > Can't we just insert a while loop here to move op_prev back across labels? > > > > while (op_next->opc == INDEX_op_set_label) { > > op_prev = QTAILQ_PREV(op, op_prev); > > } > > > >> - if (op_prev->opc == INDEX_op_br && > >> - label == arg_label(op_prev->args[0])) { > Ah I see, you're thinking something like > > dead = false; > remove = false; > > if (label->refs == 1) { > TCGOp *op_prev = NULL; > do { > op_prev = QTAILQ_PREV(op_prev, link); You can't use op_prev as the argument here. It is NULL the first time through the loop ☹ > } while (op_prev->opc == INDEX_op_set_label); > > if (op_prev->opc == INDEX_op_br && > label == arg_label(op_prev->args[0])) { > tcg_op_remove(s, op_prev); > remove = true; > } > } > > to handle > > br > set_label > ... > set_label > > I do like being able to combine both branches, and we're no longer relying on > the next iteration of the loop to clean up the final set_label. Since we won't > ever encounter more than one intermediate set_label this might be overkill, > but either way I think it's an improvement. It's overkill for the code that idef-parser generates, but my suggestion is to make able to skip back over as many labels as possible. > > Thanks for the feedback, I'll resubmit with this change! :) > > -- > Anton Johansson, > rev.ng Labs Srl.
On 3/3/23 16:39, Taylor Simpson wrote: >> dead = false; >> remove = false; >> >> if (label->refs == 1) { >> TCGOp *op_prev = NULL; >> do { >> op_prev = QTAILQ_PREV(op_prev, link); > You can't use op_prev as the argument here. It is NULL the first time through the loop ☹ Ah right TCGop *op_prev = op; should do it.
© 2016 - 2024 Red Hat, Inc.