> -----Original Message----- > From: Anton Johansson <a...@rev.ng> > Sent: Friday, March 3, 2023 7:32 AM > To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org > Cc: a...@rev.ng; richard.hender...@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.