[0/4] RISC-V: Implement TLS Descriptors.

  This patchset implements TLS Descriptors (TLSDESC) for RISC-V targets, per
the specification draft at [1].

This patchset is based on top of [2].

No regression in binutils and gcc tests for rv64gc, tested alongside the
gcc and glibc implementation (will be posted shortly).

This contribution is made on behalf of Blue Whale Systems, which has
copyright assignment on file with the FSF.

[1]: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/373
[2]: https://sourceware.org/pipermail/binutils/2023-August/129075.html

Tatsuyuki Ishi (4):
  RISC-V: Add TLSDESC reloc definitions.
  RISC-V: Add assembly support for TLSDESC.
  RISC-V: Define and use GOT entry size constants for TLS.
  RISC-V: Initial ld.bfd support for TLSDESC.

 bfd/bfd-in2.h                              |   4 +
 bfd/elfnn-riscv.c                          | 105 ++++++++++++++++++---
 bfd/elfxx-riscv.c                          |  75 ++++++++++++++-
 bfd/libbfd.h                               |   4 +
 bfd/reloc.c                                |   8 ++
 gas/config/tc-riscv.c                      |  18 +++-
 include/elf/riscv.h                        |   5 +
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |   2 +
 opcodes/riscv-opc.c                        |   1 +
 9 files changed, 201 insertions(+), 21 deletions(-)
  

Hi,

Before the reviewing, I think you still should have test cases for each
patch to make sure everything is correct in the binutils, and helps people
understand what the patches try to resolve what problem or support what
features.  In general, the regressions in binutils, gcc and glibc are
seperate.  Besides, each patch should at least have the ChangLogs in the
commit message.

Thanks
Nelson

On Fri, Aug 18, 2023 at 2:10 AM Tatsuyuki Ishi via Binutils <
binutils@sourceware.org> wrote:

> This patchset implements TLS Descriptors (TLSDESC) for RISC-V targets, per
> the specification draft at [1].
>
> This patchset is based on top of [2].
>
> No regression in binutils and gcc tests for rv64gc, tested alongside the
> gcc and glibc implementation (will be posted shortly).
>
> This contribution is made on behalf of Blue Whale Systems, which has
> copyright assignment on file with the FSF.
>
> [1]: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/373
> [2]: https://sourceware.org/pipermail/binutils/2023-August/129075.html
>
> Tatsuyuki Ishi (4):
>   RISC-V: Add TLSDESC reloc definitions.
>   RISC-V: Add assembly support for TLSDESC.
>   RISC-V: Define and use GOT entry size constants for TLS.
>   RISC-V: Initial ld.bfd support for TLSDESC.
>
>  bfd/bfd-in2.h                              |   4 +
>  bfd/elfnn-riscv.c                          | 105 ++++++++++++++++++---
>  bfd/elfxx-riscv.c                          |  75 ++++++++++++++-
>  bfd/libbfd.h                               |   4 +
>  bfd/reloc.c                                |   8 ++
>  gas/config/tc-riscv.c                      |  18 +++-
>  include/elf/riscv.h                        |   5 +
>


>  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |   2 +
>

This doesn't work, seems still not adding any test cases.


>  opcodes/riscv-opc.c                        |   1 +
>  9 files changed, 201 insertions(+), 21 deletions(-)
>
> --
> 2.41.0
>
>
  

On Thu, Aug 17, 2023 at 5:22 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
> Hi,
>
> Before the reviewing, I think you still should have test cases for each
> patch to make sure everything is correct in the binutils, and helps people
> understand what the patches try to resolve what problem or support what
> features.  In general, the regressions in binutils, gcc and glibc are
> seperate.  Besides, each patch should at least have the ChangLogs in the
> commit message.
>
> Thanks
> Nelson

A relocatable object file can contain both TLSDESC and TLS GD
relocations (e.g. relocatable output).
The GOT use cases are different. It will be nice to have such a test.

We need tests for at least (a) -shared and (b) -no-pie or -pie (I
prefer just -no-pie).

Perhaps you have done this (as the glibc patch suggests), but I want
to make sure that the GOT entries are placed in .rela.dyn, not
.rela.plt
(https://sourceware.org/bugzilla/show_bug.cgi?id=28387  ld: Move
R_*_TLSDESC to .rela.dyn)
sysdeps/{aarch64,x86_64}/dl-machine.h unfortunately have to handle
TLSDESC in elf_machine_lazy_rel for compatibility.

> [PATCH 4/4] RISC-V: Initial ld.bfd support for TLSDESC.
> + /* TLSDESC needs one dynamic reloc and four GOT slots. *

Why is there 4? Other ports reserve 2 GOT slots for one symbol.

> On Fri, Aug 18, 2023 at 2:10 AM Tatsuyuki Ishi via Binutils <
> binutils@sourceware.org> wrote:
>
> > This patchset implements TLS Descriptors (TLSDESC) for RISC-V targets, per
> > the specification draft at [1].
> >
> > This patchset is based on top of [2].
> >
> > No regression in binutils and gcc tests for rv64gc, tested alongside the
> > gcc and glibc implementation (will be posted shortly).
> >
> > This contribution is made on behalf of Blue Whale Systems, which has
> > copyright assignment on file with the FSF.
> >
> > [1]: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/373
> > [2]: https://sourceware.org/pipermail/binutils/2023-August/129075.html
> >
> > Tatsuyuki Ishi (4):
> >   RISC-V: Add TLSDESC reloc definitions.
> >   RISC-V: Add assembly support for TLSDESC.
> >   RISC-V: Define and use GOT entry size constants for TLS.
> >   RISC-V: Initial ld.bfd support for TLSDESC.
> >
> >  bfd/bfd-in2.h                              |   4 +
> >  bfd/elfnn-riscv.c                          | 105 ++++++++++++++++++---
> >  bfd/elfxx-riscv.c                          |  75 ++++++++++++++-
> >  bfd/libbfd.h                               |   4 +
> >  bfd/reloc.c                                |   8 ++
> >  gas/config/tc-riscv.c                      |  18 +++-
> >  include/elf/riscv.h                        |   5 +
> >
>
>
> >  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |   2 +
> >
>
> This doesn't work, seems still not adding any test cases.
>
>
> >  opcodes/riscv-opc.c                        |   1 +
> >  9 files changed, 201 insertions(+), 21 deletions(-)
> >
> > --
> > 2.41.0
> >
> >