RE: [PATCH v2 02/10] target/hexagon: import README for idef-parser

[Taylor Simpson <tsimpson@quicinc.com>]

> -----Original Message-----
> From: Alessandro Di Federico <ale.qemu@rev.ng>
> Sent: Thursday, March 18, 2021 12:27 PM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>; qemu-
> devel@nongnu.org; Brian Cain <bcain@quicinc.com>; babush@rev.ng;
> nizzo@rev.ng; philmd@redhat.com
> Subject: Re: [PATCH v2 02/10] target/hexagon: import README for idef-
> parser
>
> On Wed, 10 Mar 2021 15:48:14 +0000
> Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> > Which instructions require this?  There must be an attribute that we
> > could check to see if it is needed before emitting the TCG.
>
> The following should be an example of an instruction that requires
> zero-initialization:
>
>     /* S2_vsplatrh */
>     void emit_S2_vsplatrh(DisasContext *ctx, Insn *insn, Packet *pkt,
>                           TCGv_i64 RddV, TCGv_i32 RsV)
>     /*  for (i=0;i<4;i++) { fSETHALF(i,RddV, fGETHALF(0,RsV)); } } */
>     {
>       tcg_gen_movi_i64(RddV, 0);
>       for (int i = ((int64_t)0ULL); i < ((int64_t)4ULL); i++) {
>         TCGv_i32 tmp_0 = tcg_temp_new_i32();
>         tcg_gen_sextract_i32(tmp_0, RsV, ((int64_t)0ULL) * 16, 16);
>         TCGv_i64 tmp_1 = tcg_temp_new_i64();
>         tcg_gen_ext_i32_i64(tmp_1, tmp_0);
>         tcg_temp_free_i32(tmp_0);
>         tcg_gen_deposit_i64(RddV, RddV, tmp_1, i * 16, 16);
>         tcg_temp_free_i64(tmp_1);
>       }
>     }
>
> If we don't zero-initialize RddV, the deposit instruction will read
> uninitialized data.
>
> Note that, IIRC, `RddV` is not always a global variable, which could be
> safely read, but it might be an uninitialized TCGv that will be
> written to the CPU state in the commit phase.

Correct, in fact you should assume it is *always* uninitialized.

For instructions like vsplatrh, the original C semantics are OK because all parts of the result are eventually written.  So, you could detect this in the parser or manually add an attribute.  If you want to add an attribute:
1) add a line to attribs_def.h.inc
2) Figure out which macros lead to a problem and add them to calculate_attribs in hex_common.py


Thanks,
Taylor