[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v4 06/12] target/hexagon: introduce new helper functions
From: |
Taylor Simpson |
Subject: |
RE: [PATCH v4 06/12] target/hexagon: introduce new helper functions |
Date: |
Mon, 19 Apr 2021 15:00:17 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Sunday, April 18, 2021 4:31 PM
> To: Alessandro Di Federico <ale.qemu@rev.ng>; qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng; philmd@redhat.com;
> Alessandro Di Federico <ale@rev.ng>
> Subject: Re: [PATCH v4 06/12] target/hexagon: introduce new helper
> functions
>
> On 4/15/21 9:34 AM, Alessandro Di Federico wrote:
> > +void gen_store32(TCGv vaddr, TCGv src, tcg_target_long width, unsigned
> slot)
> > +{
> > + tcg_gen_mov_tl(hex_store_addr[slot], vaddr);
> > + tcg_gen_movi_tl(hex_store_width[slot], width);
> > + tcg_gen_mov_tl(hex_store_val32[slot], src);
> > +}
> > +
> > +void gen_store1(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext
> *ctx,
> > + unsigned slot)
> > +{
> > + gen_store32(vaddr, src, 1, slot);
> > + ctx->store_width[slot] = 1;
> > +}
>
> Why is store_width here and not in gen_store32?
> Do you really need so many helpers here, as opposed to making use of
> MemOp?
These are included in this patch
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01355.html
which hasn't been merged yet.
>
> > +void gen_sat_i32_ext(TCGv ovfl, TCGv dest, TCGv source, int width)
> > +{
> > + gen_sat_i32(dest, source, width);
> > + TCGv zero = tcg_const_i32(0);
> > + TCGv one = tcg_const_i32(1);
> > + tcg_gen_movcond_i32(TCG_COND_NE, ovfl, source, dest, one, zero);
>
> (source != dest ? 1 : 0) -> (source != dest).
>
> Therefore, tcg_gen_setcond_i32.
>
> Or did you intend
>
> ovfl = (source != dest ? 1 : ovfl)?
>
> which is probably still better as
>
> tcg_gen_setcond_tl(TCG_COND_NE, tmp, source,dest);
> tcg_gen_or_tl(ovfl, ovfl, tmp);
>
> > +void gen_fbrev(TCGv result, TCGv src)
> > +{
> > + TCGv lo = tcg_temp_new();
> > + TCGv tmp1 = tcg_temp_new();
> > + TCGv tmp2 = tcg_temp_new();
> > +
> > + /* Bit reversal of low 16 bits */
> > + tcg_gen_extract_tl(lo, src, 0, 16);
> > + tcg_gen_andi_tl(tmp1, lo, 0xaaaa);
> > + tcg_gen_shri_tl(tmp1, tmp1, 1);
> > + tcg_gen_andi_tl(tmp2, lo, 0x5555);
> > + tcg_gen_shli_tl(tmp2, tmp2, 1);
> > + tcg_gen_or_tl(lo, tmp1, tmp2);
> > + tcg_gen_andi_tl(tmp1, lo, 0xcccc);
> > + tcg_gen_shri_tl(tmp1, tmp1, 2);
> > + tcg_gen_andi_tl(tmp2, lo, 0x3333);
> > + tcg_gen_shli_tl(tmp2, tmp2, 2);
> > + tcg_gen_or_tl(lo, tmp1, tmp2);
> > + tcg_gen_andi_tl(tmp1, lo, 0xf0f0);
> > + tcg_gen_shri_tl(tmp1, tmp1, 4);
> > + tcg_gen_andi_tl(tmp2, lo, 0x0f0f);
> > + tcg_gen_shli_tl(tmp2, tmp2, 4);
> > + tcg_gen_or_tl(lo, tmp1, tmp2);
> > + tcg_gen_bswap16_tl(lo, lo);
> > +
> > + /* Final tweaks */
> > + tcg_gen_deposit_tl(result, src, lo, 0, 16);
> > + tcg_gen_or_tl(result, result, lo);
> > +
> > + tcg_temp_free(lo);
> > + tcg_temp_free(tmp1);
> > + tcg_temp_free(tmp2);
> > +}
>
> Coordinate with Taylor.
> https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg10007.html
Once this patch series is merged, many load/store instructions will have manual
overrides. I can easily provide overrides for the remainder. Then, we could
skip them in the idef-parser. At a minimum, you should skip the ones that are
part of that series
- circular addressing
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01355.html
- bit reverse addressing
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01354.html
- load and unpack bytes
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01353.html
- load into shifted register
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01359.html
Alessandro, what do you think?
Thanks,
Taylor
- [PATCH v4 04/12] target/hexagon: make slot number an unsigned, (continued)
- [PATCH v4 04/12] target/hexagon: make slot number an unsigned, Alessandro Di Federico, 2021/04/15
- [PATCH v4 02/12] target/hexagon: update MAINTAINERS for idef-parser, Alessandro Di Federico, 2021/04/15
- [PATCH v4 01/12] tcg: expose TCGCond manipulation routines, Alessandro Di Federico, 2021/04/15
- [PATCH v4 07/12] target/hexagon: expose next PC in DisasContext, Alessandro Di Federico, 2021/04/15
- [PATCH v4 06/12] target/hexagon: introduce new helper functions, Alessandro Di Federico, 2021/04/15
- [PATCH v4 03/12] target/hexagon: import README for idef-parser, Alessandro Di Federico, 2021/04/15
- [PATCH v4 12/12] target/hexagon: import additional tests, Alessandro Di Federico, 2021/04/15
- [PATCH v4 09/12] target/hexagon: import lexer for idef-parser, Alessandro Di Federico, 2021/04/15
[PATCH v4 08/12] target/hexagon: prepare input for the idef-parser, Alessandro Di Federico, 2021/04/15