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

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Sunday, February 14, 2021 6:07 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; alex.bennee@linaro.org; laurent@vivier.eu;
> ale@rev.ng; Brian Cain <bcain@quicinc.com>
> Subject: Re: [PATCH v8 26/35] Hexagon (target/hexagon) TCG generation
>
> 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.

Note that slot_mask indicates that the value is not written (i.e., slot was 
cancelled).  So, the code will be
    tcg_gen_xori_tl(slot_mask, slot_mask, 1 << slot);
    tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], slot_mask);


> > +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?

The instruction semantics assume the values will be truncated to 8 bits when 
written.  Here's an example:
    Q6INSN(C2_not,"Pd4=not(Ps4)",ATTRIBS(A_CRSLOT23),
    "Logical NOT Predicate",
    {PdV=~PsV;})

BTW, how can I generate an assert via TCG?


> > +    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.

It's legal to write to a predicate more than once in the same packet.  In this 
case, we and the results together.  This is described in Section 6.1.3 of the 
reference manual.  I'll beef up the comment to make it more clear.


Thanks,
Taylor