Re: [PATCH] RISC-V: Add %got_pcrel_hi operator

[Jim Wilson <jimw at sifive dot com>]

On 11/7/18 10:27 AM, James Clarke wrote:
> This operator allows compilers to expand the "la" pseduo-instruction and
> potentially split up the two instructions.
>
> gas/
> 	* config/tc-riscv.c (percent_op_utype): Add entry for a new
> 	%got_pcrel_hi operator.
> 	* testsuite/gas/riscv/no-relax-reloc.d: Update for new
> 	%got_pcrel_hi operator test.
> 	* testsuite/gas/riscv/no-relax-reloc.s: Add a %got_pcrel_hi
> 	operator test.
> 	* testsuite/gas/riscv/relax-reloc.d: Update for new
> 	%got_pcrel_hi operator test.
> 	* testsuite/gas/riscv/relax-reloc.s: Add a %got_pcrel_hi
> 	operator test.

We have a list of assembler operators in the psABI document
    https://github.com/riscv/riscv-elf-psabi-doc
We also have a list of assembler operators in the assembler manual
    https://github.com/riscv/riscv-asm-manual
This looks a little redundant.  It isn't clear why assembler syntax is 
mentioned in the psABI.  It doesn't seem relevant to that.  Anyways, 
these should be updated.

It is probably best if there is a public proposal somewhere on the 
RISC-V side instead of unilaterally changing assembler syntax, 
particularly given that it is part of the psABI (even though it probably 
shouldn't be).  Someone on the LLVM side for instance might want to 
offer an opinion.  An issue filed with the psABI or assembler manual, or 
an email sent to sw-dev@riscv.org is probably sufficient.

Maybe the operator could just be %got_hi?  Not clear why we need the 
pcrel in there.  The reloc itself is just R_RISCV_GOT_HI20.

Do you have a copyright assignment for binutils?  I found ones for mach, 
hurd, and glibc.  I don't see an obvious binutils one.

Otherwise this looks OK to me.

Jim