Re: [Qemu-devel] [PATCH 2/9] tcg: Add INDEX_op_extract2_{i32,i64}

[Peter Maydell]

On Wed, 3 Apr 2019 at 18:37, Richard Henderson
<address@hidden> wrote:
>
> On 3/26/19 8:35 PM, Peter Maydell wrote:
> > On Thu, 7 Mar 2019 at 14:47, Richard Henderson
> > <address@hidden> wrote:
> >>
> >> This will let backends implement the double-word shift operation.
> >>
> >> Signed-off-by: Richard Henderson <address@hidden>
> >> diff --git a/tcg/README b/tcg/README
> >> index 603f4df659..ddabf33017 100644
> >> --- a/tcg/README
> >> +++ b/tcg/README
> >> @@ -343,6 +343,11 @@ at bit 8.  This operation would be equivalent to
> >>
> >>  (using an arithmetic right shift).
> >>
> >> +* extract2_i64 dest, t1, t2, pos
> >> +
> >> +Extract a 64-bit quantity from the concatenation of t2:t1,
> >> +beginning at pos.
> >> +
> >
> > I think we should document the valid values of 'pos'.
> > My guess is "0 <= pos <= 63".
>
> How about
>
> ----
> * extract2_i32/i64 dest, t1, t2, pos
>
> For N = {32,64}, extract an N-bit quantity from the concatenation
> of t2:t1, beginning at pos.  The tcg_gen_extract2_* expander allows
> values 0 <= pos <= N, but will expand 0 and N with mov, so only
> 1 <= pos <= N-1 will be seen by the host tcg_out_op.

If I'm reading that correctly, it seems to be combining in one sentence
the behaviour of the TCG API exposed to the front-end (pos can be
between 0 and N inclusive) with a detail of the API that a backend
needs to care about (that it can assume it never sees 0 or N).
I think we should be more careful to keep those separate, because
a reader of this document is almost always going to care only about
one or the other, never both at the same time. Perhaps things that
apply only to the backend end of the interface should go in section 4
of tcg/README? At any rate I think they should at least be in
different sentences :-)

thanks
-- PMM