Re: [PATCH 1/1] tcg: convert tcg/README to rst

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 24/11/2022 13:21, Peter Maydell wrote:

> On Wed, 23 Nov 2022 at 09:54, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>>
>> Convert tcg/README to rst and move it to docs/devel as a new "TCG Intermediate
>> Representation" page. There are a few minor changes to improve the aesthetic
>> of the final output which are as follows:
>>
>>    - Rename the title from "Tiny Code Generator - Fabrice Bellard" to "TCG
>>      Intermediate Representation"
>>
>>    - Remove the section numbering
>>
>>    - Add the missing parameters to the ssadd_vec operations in the "Host
>>      vector operations" section
>>
>>    - Change the path to the Atomic Operations document to use a proper
>>      reference
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> 
>> +   * - ctpop_i32/i64 *t0*, *t1*
>> +
>> +     - .. line-block::
>> +          *t0* = number of bits set in *t1*
>> +
>> +          With *ctpop* short for "count population", matching
>> +          the function name used in ``include/qemu/host-utils.h``.
> 
> Because we're using line-block, any newline is a literal newline.
> This causes some odd looking linebreaks because you get a linebreak
> both where it is in the input (after "matching") and again when
> the browser auto-wraps long lines. I think it would be better
> to not use line-block at all. Instead, we can just put in a blank
> line when we want a line break, like this:
> 
> diff --git a/docs/devel/tcg-ops.rst b/docs/devel/tcg-ops.rst
> index 0d90f3f7a00..e8a51ed1459 100644
> --- a/docs/devel/tcg-ops.rst
> +++ b/docs/devel/tcg-ops.rst
> @@ -252,8 +252,9 @@ Arithmetic
> 
>      * - rem_i32/i64 *t0*, *t1*, *t2*
> 
> -     - .. line-block::
> +     -
>             *t0* = *t1* % *t2* (signed)
> +
>             Undefined behavior if division by zero or overflow.
> 
>      * - remu_i32/i64 *t0*, *t1*, *t2*
> @@ -339,8 +340,9 @@ Shifts/Rotates
> 
>      * - shl_i32/i64 *t0*, *t1*, *t2*
> 
> -     - .. line-block::
> +     -
>             *t0* = *t1* << *t2*
> +
>             Unspecified behavior if *t2* < 0 or *t2* >= 32 (resp 64)
> 
>      * - shr_i32/i64 *t0*, *t1*, *t2*
> 
> This will result in slightly more whitespace between lines
> in a few places, but I think it's nicer than the mis-wrapped
> paragraphs.

I tried this locally, however without the explicit line-block:: everything was still 
placed in a single line even with multiple blank lines. I think I read somewhere that 
all table content is forced into a single document by default, which explains why it 
was necessary for me to add the line-block:: to get multi-line table content.

> If you really want to keep the line-block formatting, I think
> you need to use the vertical-bar version, which lets you
> specify which lines of the source are really new lines
> (they have a '|' at the start), and which are just continuations
> of the same line (they start with a ' ' instead), like this:
> 
> @@ -325,11 +325,11 @@ Logical
> 
>      * - ctpop_i32/i64 *t0*, *t1*
> 
> -     - .. line-block::
> -          *t0* = number of bits set in *t1*
> -
> -          With *ctpop* short for "count population", matching
> -          the function name used in ``include/qemu/host-utils.h``.
> +     -
> +       |   *t0* = number of bits set in *t1*
> +       |
> +       |   With *ctpop* short for "count population", matching
> +           the function name used in ``include/qemu/host-utils.h``.
> 
> 
> But I'm not sure it's really worth the effort.

This approach seems to work well for me, certainly it feels easier to maintain 
compared to switching everything over to use grid tables. I've got a v2 update which 
I shall post shortly.

> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM


ATB,

Mark.