Re: [PATCH 2/2] RISC-V: Support assembler modifier %got_pcrel_hi.

[Fangrui Song <i@maskray.me>]

On 2020-03-03, Jim Wilson wrote:
>On Tue, Mar 3, 2020@2:22 AM Nelson Chu <nelson.chu@sifive.com> wrote:
>>
>>         gas/
>>         * config/tc-riscv.c: Support the modifier %got_pcrel_hi.
>>         * doc/c-riscv.texi: Add documentation.
>>         * testsuite/gas/riscv/no-relax-reloc.d: Add test case for the new
>>         modifier %got_pcrel_hi.
>>         * testsuite/gas/riscv/no-relax-reloc.s: Likewise.
>>         * testsuite/gas/riscv/relax-reloc.d: Likewise.
>>         * testsuite/gas/riscv/relax-reloc.s: Likewise.
>
>This looks good.  I'd just suggest some minor doc fixes to be
>consistent with suggestions for the first patch.
>
>> +@item %got_pcrel_hi(@var{symbol}
>> +The high 20-bit of relative address between pc and the GOT entry of
>> +@var{symbol}.  This is usually used with the %pcrel_lo to access the GOT entry.
>
>20-bit -> 20 bits
>"the %pcrel_lo" -> "%pcrel_lo" or "the %pcrel_lo modifier"
>
>> +@smallexample
>> +@var{label}:
>> +       auipc           a0, %got_pcrel_hi(@var{symbol}) // R_RISCV_GOT_HI20
>> +       addi/load/store a0, a0, %pcrel_lo(@var{label})  // R_RISCV_PCREL_LO12_I/S
>> +@end smallexample
>> +
>> +Also, the pseudo la instruction with PIC has the similar behavior.
>
>"the similar" -> "similar" or "the same"
>
>Jim

What if %got_pcrel_hi refers to a STB_LOCAL symbol? For example,

auipc a0, %got_pcrel_hi(.L1)

It may be worth a test.

FWIW, llvm-mc -triple=riscv64 -filetype=obj a.s # create a R_RISCV_GOT_HI20 referencing .L1
There is no special treatment.