Taylor Simpson <tsimp...@quicinc.com> wrote:
> diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
> index b56b216110..dbafcae2de 100644
> --- a/target/hexagon/gen_tcg.h
> +++ b/target/hexagon/gen_tcg.h
>
> +#define fGEN_TCG_J4_cmplt_f_jumpnv_t(SHORTCODE) \
> + gen_cmp_jumpnv(ctx, pkt, TCG_COND_GE, NsN, RtV, riV)
> +#define fGEN_TCG_J4_cmplt_f_jumpnv_nt(SHORTCODE) \
> + gen_cmp_jumpnv(ctx, pkt, TCG_COND_GE, NsN, RtV, riV)
> +
Nitpick: any reason not to place these two !COND variants...
> +
> +#define fGEN_TCG_J4_cmplt_t_jumpnv_t(SHORTCODE) \
> + gen_cmp_jumpnv(ctx, pkt, TCG_COND_LT, NsN, RtV, riV)
> +#define fGEN_TCG_J4_cmplt_t_jumpnv_nt(SHORTCODE) \
> + gen_cmp_jumpnv(ctx, pkt, TCG_COND_LT, NsN, RtV, riV)
> +
...below these COND variants, like you did in the other insns?
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 6e494c0bd8..fba76d3b38 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -456,6 +456,35 @@ static TCGv gen_8bitsof(TCGv result, TCGv value)
> return result;
> }
>
> +static void gen_write_new_pc_addr(DisasContext *ctx, Packet *pkt,
> + TCGv addr, TCGv pred)
> +{
> + TCGLabel *pred_false = NULL;
> + if (pkt->pkt_has_multi_cof) {
> + if (pred != NULL) {
> + pred_false = gen_new_label();
> + tcg_gen_brcondi_tl(TCG_COND_EQ, pred, 0, pred_false);
> + }
> + /* If there are multiple branches in a packet, ignore the second one
> */
> + tcg_gen_movcond_tl(TCG_COND_NE, hex_gpr[HEX_REG_PC],
> + hex_branch_taken, tcg_constant_tl(0),
> + hex_gpr[HEX_REG_PC], addr);
> + tcg_gen_movi_tl(hex_branch_taken, 1);
> + if (pred != NULL) {
> + gen_set_label(pred_false);
> + }
> + } else {
> + if (pred != NULL) {
> + pred_false = gen_new_label();
> + tcg_gen_brcondi_tl(TCG_COND_EQ, pred, 0, pred_false);
> + }
> + tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], addr);
> + if (pred != NULL) {
> + gen_set_label(pred_false);
> + }
> + }
Another nitpick: we could possibly extract the common code out of the
if-else branches for clarity (but note my other comment about this
function at patch 7):
static void gen_write_new_pc_addr(DisasContext *ctx, Packet *pkt,
TCGv addr, TCGv pred)
{
TCGLabel *pred_false = NULL;
if (pred != NULL) {
pred_false = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, pred, 0, pred_false);
}
if (pkt->pkt_has_multi_cof) {
/* If there are multiple branches in a packet, ignore the second one */
tcg_gen_movcond_tl(TCG_COND_NE, hex_gpr[HEX_REG_PC],
hex_branch_taken, tcg_constant_tl(0),
hex_gpr[HEX_REG_PC], addr);
tcg_gen_movi_tl(hex_branch_taken, 1);
} else {
tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], addr);
}
if (pred != NULL) {
gen_set_label(pred_false);
}
}
> --
>2.17.1
The rest of the patch LGTM.
