Re: [PATCH v8 26/35] Hexagon (target/hexagon) TCG generation

[Richard Henderson]

On 2/7/21 9:46 PM, Taylor Simpson wrote:
> +static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)

Drop all the inline markup.  Let the compiler decide.  It also means we get
more consistent warnings for unused functions between gcc and clang (clang will
warn for unused inlines outside a header file, gcc will not).

Also, quite a few of these function are really way too big to want to inline.

> +{
> +    TCGv one = tcg_const_tl(1);
> +    TCGv zero = tcg_const_tl(0);
> +    TCGv slot_mask = tcg_temp_new();
> +
> +    tcg_gen_andi_tl(slot_mask, hex_slot_cancelled, 1 << slot);
> +    tcg_gen_movcond_tl(TCG_COND_EQ, hex_new_value[rnum], slot_mask, zero,
> +                           val, hex_new_value[rnum]);
> +#if HEX_DEBUG
> +    /* Do this so HELPER(debug_commit_end) will know */
> +    tcg_gen_movcond_tl(TCG_COND_EQ, hex_reg_written[rnum], slot_mask, zero,
> +                       one, hex_reg_written[rnum]);

This is a very complicated OR.

If you really want hex_reg_written = {0,1}, then change to

    tcg_gen_extract_tl(slot_mask, hex_slot_canceled, slot, 1);

otherwise, just let hex_reg_written be zero/non-zero, and or in the slot.

> +static inline void gen_log_pred_write(int pnum, TCGv val)
> +{
> +    TCGv zero = tcg_const_tl(0);
> +    TCGv base_val = tcg_temp_new();
> +    TCGv and_val = tcg_temp_new();
> +    TCGv pred_written = tcg_temp_new();
> +
> +    /* Multiple writes to the same preg are and'ed together */
> +    tcg_gen_andi_tl(base_val, val, 0xff);

Why is this here, rather than asserting that we never generate an out-of-range
value?

> +    tcg_gen_and_tl(and_val, base_val, hex_new_pred_value[pnum]);
> +    tcg_gen_andi_tl(pred_written, hex_pred_written, 1 << pnum);
> +    tcg_gen_movcond_tl(TCG_COND_NE, hex_new_pred_value[pnum],
> +                       pred_written, zero,
> +                       and_val, base_val);
> +    tcg_gen_ori_tl(hex_pred_written, hex_pred_written, 1 << pnum);

Except for HEX_DEBUG, why would we not know whether or not a predicate has been
written twice?  It seems like we shouldn't need hex_pred_written for the
non-debug case.


r~