Re: [PATCH 1/2] RISC-V: Add description for RISC-V Modifiers to as doc.

[Jim Wilson <jimw@sifive.com>]

On Tue, Mar 3, 2020 at 2:22 AM Nelson Chu <nelson.chu@sifive.com> wrote:

Overall I think this looks good, it just needs a few typos fixed and
some improvement from a native English speaker.

> +RISC-V assembler supports following modifiers for relocatable address

I'd suggest "The RISC-V assembler supports the following modifiers for
relocatable addresses".

> +used in RISC-V instruction operands.  However, we also support some
> +pseudo instructions to reduce the use of error-prone for these modifiers.

"the use of error-prone for these modifiers" is awkward.  I'd suggest
"error-prone usage of these modifiers".  Or alternatively reword the
sentence, this could be something like "However, we also support some
pseudo instructions that are easier to use than these modifiers"

> +
> +@table @code
> +@item %lo(@var{symbol})
> +The low 12-bit of absolute address for @var{symbol}.

The general rule is that if it is an adjective if is hyphenated and
singular, but if it is a noun it is unhyphenated and plural.  At least
this is the gcc doc rule, I don't know if binutils handles it
differently.  So this should be "low 12 bits" or maybe "low 12-bit
value", but the first one is fine.

> +
> +@item %hi(@var{symbol})
> +The high 20-bit of absolute address for @var{symbol}.  This is usually
> +used with the %lo to represent a 32-bit absolute address.

"high 20 bits" but "32-bit" is correct.
"the %lo" looks awkward, I'd suggest "%lo" or "the %lo modifier"

> +
> +@smallexample
> +       lui             a0, %hi(@var{symbol})     // R_RISCV_HI20
> +       addi/load/store a0, a0, %lo(@var{symbol}) // R_RISCV_LO12_I/S
> +@end smallexample
> +
> +@item %pcrel_lo(@var{label})
> +The low 12-bit of relative address between pc and @var{symbol}.
> +The @var{symbol} is related to the high part instrcution which marked
> +by @var{label}.

12-bit -> 12 bits
instrcution -> instruction
"which marked" -> "which is marked"

> +
> +@item %pcrel_hi(@var{symbol})
> +The high 20-bit of relative address between pc and @var{symbol}.
> +This is usually used with the %pcrel_lo to represent a 4GB pc-relative range.

20-bit -> 20 bits
the %pcrel_lo -> %pcrel_lo or "the %pcrel_lo modifier"

The range is signed.  You might want to mention that, maybe use +/-2GB
instead of 4GB.

> +
> +@smallexample
> +@var{label}:
> +       auipc           a0, %pcrel_hi(@var{symbol})    // R_RISCV_PCREL_HI20
> +       addi/load/store a0, a0, %pcrel_lo(@var{label}) // R_RISCV_PCREL_LO12_I/S
> +@end smallexample
> +
> +Or you can use the pseudo lla/lw/sw/... instruction to do this.
> +
> +@smallexample
> +       lla  a0, @var{symbol}
> +@end smallexample
> +
> +@item %tprel_add(@var{symbol}
> +This is used purely to associate the R_RISCV_TPREL_ADD relocation for
> +TLS relaxation.

This one is only valid as the fourth operand to the normally 3 operand
add instruction.  It is probably worth mentioning that.  I see you do
have an example below, but I think it should still be pointed out.

> +
> +@item %tprel_lo(@var{symbol})
> +The low 12-bit of relative address between tp and @var{symbol}

12-bit -> 12 bits

> +
> +@item %tprel_hi(@var{symbol})
> +The high 20-bit of relative address between tp and @var{symbol}.  This is
> +usually used with the %tprel_lo and %tprel_add to access the thread local
> +variable @var{symbol} in TLS Local Exec.

20-bit -> 20 bits
"the %tprel_lo and %tprel_add" -: "%tprel_lo and %tprel_add" or "the
%tprel_lo and %tprel_add modifiers"

> +
> +@smallexample
> +       lui        a5, %tprel_hi(@var{symbol})          // R_RISCV_TPREL_HI20
> +       add        a5, a5, tp, %tprel_add(@var{symbol}) // R_RISCV_TPREL_ADD
> +       load/store t0, %tprel_lo(@var{symbol})(a5)      // R_RISCV_TPREL_LO12_I/S
> +@end smallexample
> +
> +@item %tls_ie_pcrel_hi(@var{symbol})
> +The high 20-bit of relative address between pc and GOT entry.  It is usually
> +used with the %pcrel_lo to access the thread local variable @var{symbol} in
> +TLS Inital Exec.

20-bit -> 20 bits
"the %pcrel_lo" -> "%pcrel_lo" or "the %pcrel_lo modifier"
Inital -> Initial

> +
> +@smallexample
> +       la.tls.ie  a5, @var{symbol}
> +       add        a5, a5, tp
> +       load/store t0, 0(a5)
> +@end smallexample
> +
> +The pseudo la.tls.ie instruction can be expended to
> +
> +@smallexample
> +@var{label}:
> +       auipc a5, %tls_ie_pcrel_hi(@var{symbol}) // R_RISCV_TLS_GOT_HI20
> +       ld/lw a5, %pcrel_lo(@var{label})(a5)     // R_RISCV_PCREL_LO12_I/S
> +@end smallexample
> +
> +@item %tls_gd_pcrel_hi(@var{symbol})
> +The high 20-bit of relative address between pc and GOT entry.  It is usually
> +used with the %pcrel_lo to access the thread local variable @var{symbol} in
> +TLS Global Dynamic.

20-bit -> 20 bits
"the %pcrel_lo" -> "%pcrel_lo" or "the %pcrel_lo modifier"

> +
> +@smallexample
> +       la.tls.gd  a0, @var{symbol}
> +       call       __tls_get_addr@@plt
> +       mv         a5, a0
> +       load/store t0, 0(a5)
> +@end smallexample
> +
> +The pseudo la.tls.gd instruction can be expended to
> +
> +@smallexample
> +@var{label}:
> +       auipc a0, %tls_gd_pcrel_hi(@var{symbol}) // R_RISCV_TLS_GD_HI20
> +       addi  a0, a0, %pcrel_lo(@var{label})     // R_RISCV_PCREL_LO12_I/S
> +@end smallexample
> +
> +@end table
> +
>  @node RISC-V-Formats
> -@section Instruction Formats
> +@section RISC-V Instruction Formats
>  @cindex instruction formats, risc-v
>  @cindex RISC-V instruction formats
>
> --
> 2.7.4
>

Jim