Re: [RFC PATCH v3 28/34] Hexagon (target/hexagon) TCG generation helpers

[Richard Henderson <richard.henderson@linaro.org>]

On 8/18/20 8:50 AM, Taylor Simpson wrote:
> Helpers for reading and writing registers
> Helpers for load-locked/store-conditional
> 
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>  target/hexagon/genptr_helpers.h | 244 ++++++++++++++++++++++++++++++++++++++++
>  target/hexagon/op_helper.c      |  18 +++
>  2 files changed, 262 insertions(+)
>  create mode 100644 target/hexagon/genptr_helpers.h
> 
> diff --git a/target/hexagon/genptr_helpers.h b/target/hexagon/genptr_helpers.h
> new file mode 100644
> index 0000000..ffcb1e3
> --- /dev/null
> +++ b/target/hexagon/genptr_helpers.h
> @@ -0,0 +1,244 @@
> +/*
> + *  Copyright(c) 2019-2020 Qualcomm Innovation Center, Inc. All Rights Reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HEXAGON_GENPTR_HELPERS_H
> +#define HEXAGON_GENPTR_HELPERS_H
> +
> +#include "tcg/tcg.h"
> +
> +static inline TCGv gen_read_reg(TCGv result, int num)
> +{
> +    tcg_gen_mov_tl(result, hex_gpr[num]);
> +    return result;
> +}
> +
> +static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
> +{
> +    tcg_gen_mov_tl(pred, hex_pred[num]);
> +    return pred;
> +}
> +
> +static inline void gen_log_reg_write(int rnum, TCGv val, int slot,
> +                                     int is_predicated)

These are quite large.  Why are they marked inline?

> +        /* Low word */
> +        tcg_gen_extrl_i64_i32(val32, val);
> +        tcg_gen_mov_tl(hex_new_value[rnum], val32);

Why are you extracting into a temporary?
This could be done with

    tcg_gen_extr_i64_i32(hex_new_value[rnum],
                         hex_new_value[rnum + 1], val);

> +static inline void gen_read_p3_0(TCGv control_reg)
> +{
> +    TCGv pval = tcg_temp_new();
> +    int i;
> +    tcg_gen_movi_tl(control_reg, 0);
> +    for (i = NUM_PREGS - 1; i >= 0; i--) {
> +        tcg_gen_shli_tl(control_reg, control_reg, 8);
> +        tcg_gen_andi_tl(pval, hex_pred[i], 0xff);
> +        tcg_gen_or_tl(control_reg, control_reg, pval);

tcg_gen_deposit_tl(control_reg, control_reg,
                   hex_pred[i], i * 8, 8);

> +    for (i = 0; i < NUM_PREGS; i++) {
> +        tcg_gen_andi_tl(pred_val, control_reg, 0xff);
> +        tcg_gen_mov_tl(hex_pred[i], pred_val);
> +        tcg_gen_shri_tl(control_reg, control_reg, 8);

tcg_gen_extract_tl(hex_pred[i], control_reg, i * 8, 8);

> +static inline void log_store32(CPUHexagonState *env, target_ulong addr,
> +                               int32_t val, int width, int slot)
> +{
> +    HEX_DEBUG_LOG("log_store%d(0x%x, %d [0x%x])\n", width, addr, val, val);
> +    env->mem_log_stores[slot].va = addr;
> +    env->mem_log_stores[slot].width = width;
> +    env->mem_log_stores[slot].data32 = val;
> +}
> +
> +static inline void log_store64(CPUHexagonState *env, target_ulong addr,
> +                               int64_t val, int width, int slot)
> +{
> +    HEX_DEBUG_LOG("log_store%d(0x%x, %ld [0x%lx])\n", width, addr, val, val);
> +    env->mem_log_stores[slot].va = addr;
> +    env->mem_log_stores[slot].width = width;
> +    env->mem_log_stores[slot].data64 = val;
> +}

... or fold this re-addition back into where it was accidentally removed.  ;-)


r~