RE: [PATCH v2 17/21] Hexagon (target/hexagon) circular addressing

[Taylor Simpson]

> -----Original Message-----
> From: Taylor Simpson
> Sent: Tuesday, April 6, 2021 10:12 PM
> To: Richard Henderson <richard.henderson@linaro.org>; qemu-
> devel@nongnu.org
> Cc: philmd@redhat.com; ale@rev.ng; Brian Cain <bcain@quicinc.com>
> Subject: RE: [PATCH v2 17/21] Hexagon (target/hexagon) circular addressing
>
>
>
> > -----Original Message-----
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Sent: Tuesday, April 6, 2021 5:12 PM
> > To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> > Cc: philmd@redhat.com; ale@rev.ng; Brian Cain <bcain@quicinc.com>
> > Subject: Re: [PATCH v2 17/21] Hexagon (target/hexagon) circular
> addressing
> >
> > On 3/31/21 8:53 PM, Taylor Simpson wrote:
> > > +static inline TCGv gen_read_reg(TCGv result, int num)
> >
> > The unnecessary inlines are back, just after having removed them in patch
> 2.
>
> The ones in genptr.c need to stay so we can switch between the overrides
> and helpers and eventually the idef-parser.
>
>
> > > +#ifdef QEMU_GENERATE
> > > +static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift)
> > > +{
> > > +    /*
> > > +     * Section 2.2.4 of the Hexagon V67 Programmer's Reference Manual
> > > +     *
> > > +     *  The "I" value from a modifier register is divided into two pieces
> > > +     *      LSB         bits 23:17
> > > +     *      MSB         bits 31:28
> > > +     * At the end we shift the result according to the shift argument
> > > +     */
> > > +    TCGv msb = tcg_temp_new();
> > > +    TCGv lsb = tcg_temp_new();
> > > +
> > > +    tcg_gen_extract_tl(lsb, val, 17, 7);
> > > +    tcg_gen_extract_tl(msb, val, 28, 4);
> > > +    tcg_gen_movi_tl(result, 0);
> > > +    tcg_gen_deposit_tl(result, result, lsb, 0, 7);
> > > +    tcg_gen_deposit_tl(result, result, msb, 7, 4);
> > > +
> > > +    tcg_gen_shli_tl(result, result, shift);
> >
> > This doesn't match
> >
> > > +#define fREAD_IREG(VAL) \
> > > +    (fSXTN(11, 64, (((VAL) & 0xf0000000) >> 21) | ((VAL >> 17) & 0x7f)))
> >
> > which has a sign-extension of the result.
> >
> >      tcg_gen_extract_tl(lsb, val 17, 7);
> >      tcg_gen_sari_tl(msb, val, 21);
> >      tcg_gen_deposit_tl(result, msb, lsb, 0, 7);
>
> Good catch - this is strange.  The value gets passed to as the "M" argument to
> HELPER(fcircadd).  The code there does
>     int32_t K_const = (M >> 24) & 0xf;
>     int32_t length = M & 0x1ffff;
>
> I will consult with the architects on this.

Nevermind, this goes to the offset argument (i.e., the number of bytes to move 
the pointer), not "M".  The offset can be negative, so I need to fix it.

Thanks,
Taylor