[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/28] tcg: Add flags argument to bswap opcodes
From: |
Peter Maydell |
Subject: |
Re: [PATCH 01/28] tcg: Add flags argument to bswap opcodes |
Date: |
Mon, 21 Jun 2021 15:15:11 +0100 |
On Mon, 21 Jun 2021 at 15:02, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/21/21 6:51 AM, Peter Maydell wrote:
> > On Mon, 21 Jun 2021 at 14:41, Peter Maydell <peter.maydell@linaro.org>
> > wrote:
> >>
> >> On Mon, 14 Jun 2021 at 09:43, Richard Henderson
> >> <richard.henderson@linaro.org> wrote:
> >>>
> >>> This will eventually simplify front-end usage, and will allow
> >>> backends to unset TCG_TARGET_HAS_MEMORY_BSWAP without loss of
> >>> optimization.
> >>>
> >>> The argument is added during expansion, not currently exposed
> >>> to the front end translators. Non-zero values are not yet
> >>> supported by any backends.
> >>
> >> Here we say non-zero values are not yet supported by the backend...
> >
> > Looking at the tcg/README docs, I think what you mean is that
> > at the moment all the backends assume/require that the caller passes
> > one of TCG_BSWAP_IZ or (TCG_BSWAP_IZ | TCG_BSWAP_OZ), since the
> > pre-flags implementation requires the top bytes to be zero and leaves
> > them that way.
>
> Correct.
>
> > But then the parts of your patch that pass in a zero
> > flags word would be wrong...
>
> The parts that pass in a zero flags word are covered by, from the README:
>
> > The flags are ignored -- the argument is present
> > for consistency with the smaller bswaps.
Ah, I see. If you fix up the commit message, maybe something like
# The argument is added during expansion, not currently exposed
# to the front end translators. The backends currently only support
# a flags value of either TCG_BSWAP_IZ, or (TCG_BSWAP_IZ | TCG_BSWAP_OZ),
# since they all require zero top bytes and leave them that way.
# At the existing callsites we pass in (TCG_BSWAP_IZ | TCG_BSWAP_OZ),
# except for the flags-ignored cases of a 32-bit swap of a 32-bit
# value and or a 64-bit swap of a 64-bit value, where we pass 0.
then
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
It would be nice to document the actual flag values/names in
the user-facing documentation, too.
thanks
-- PMM
- Re: [PATCH 04/28] tcg/arm: Support bswap flags, (continued)
[PATCH 02/28] tcg/i386: Support bswap flags, Richard Henderson, 2021/06/14
[PATCH 06/28] tcg/ppc: Split out tcg_out_sari{32,64}, Richard Henderson, 2021/06/14
[PATCH 08/28] tcg/ppc: Split out tcg_out_bswap32, Richard Henderson, 2021/06/14
[PATCH 07/28] tcg/ppc: Split out tcg_out_bswap16, Richard Henderson, 2021/06/14