Re: [PATCH v5] RISC-V: Implement TLS Descriptors.

[Kito Cheng <kito.cheng@gmail.com>]

Hi Tatsuyuki:

Thanks for your hard work and keep updating, the patch set is LGTM, I
plan to commit this next week if no further comments :)

Hi MaskRay:

Thanks for your review on the patchset! just put you into the cc list
in case you have few more comments :)


On Fri, Mar 29, 2024 at 1:53 PM Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
>
> This implements TLS Descriptors (TLSDESC) as specified in [1].
>
> The 4-instruction sequence is implemented as a single RTX insn for
> simplicity, but this can be revisited later if instruction scheduling or
> more flexible RA is desired.
>
> The default remains to be the traditional TLS model, but can be configured
> with --with-tls={trad,desc}. The choice can be revisited once toolchain
> and libc support ships.
>
> [1]: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/373.
>
> gcc/Changelog:
>         * config/riscv/riscv.opt: Add -mtls-dialect to configure TLS flavor.
>         * config.gcc: Add --with-tls configuration option to change the
>         default TLS flavor.
>         * config/riscv/riscv.h: Add TARGET_TLSDESC determined from
>         -mtls-dialect and with_tls defaults.
>         * config/riscv/riscv-opts.h: Define enum riscv_tls_type for the
>         two TLS flavors.
>         * config/riscv/riscv-protos.h: Define SYMBOL_TLSDESC symbol type.
>         * config/riscv/riscv.md: Add instruction sequence for TLSDESC.
>         * config/riscv/riscv.cc (riscv_symbol_insns): Add instruction
>         sequence length data for TLSDESC.
>         (riscv_legitimize_tls_address): Add lowering of TLSDESC.
>         * doc/install.texi: Document --with-tls for RISC-V.
>         * doc/invoke.texi: Document -mtls-dialect for RISC-V.
>         * testsuite/gcc.target/riscv/tls_1.x: Add TLSDESC GD test case.
>         * testsuite/gcc.target/riscv/tlsdesc.c: Same as above.
> ---
> No regression in gcc tests for rv32gcv and rv64gcv, tested alongside
> the binutils and glibc implementation. Tested with --with-tls=desc.
>
> v2: Add with_tls configuration option, and a few readability improvements.
>     Added Changelog.
> v3: Add documentation per Kito's suggestion.
>     Fix minor issues pointed out by Kito and Jeff.
>     Thanks Kito Cheng and Jeff Law for review.
> v4: Add TLSDESC GD assembly test.
>     Rebase on top of trunk.
> v5: Trivial rebase on top of trunk.
>
> I have recently addressed relaxation concerns on binutils and RVV
> register save/restore on glibc, so I'm sending out a trivial rebase
> with the hope that the full set can be merged soon.
>
>  gcc/config.gcc                           | 15 ++++++++++++++-
>  gcc/config/riscv/riscv-opts.h            |  6 ++++++
>  gcc/config/riscv/riscv-protos.h          |  5 +++--
>  gcc/config/riscv/riscv.cc                | 24 ++++++++++++++++++++----
>  gcc/config/riscv/riscv.h                 |  9 +++++++--
>  gcc/config/riscv/riscv.md                | 20 +++++++++++++++++++-
>  gcc/config/riscv/riscv.opt               | 14 ++++++++++++++
>  gcc/doc/install.texi                     |  3 +++
>  gcc/doc/invoke.texi                      | 13 ++++++++++++-
>  gcc/testsuite/gcc.target/riscv/tls_1.x   |  5 +++++
>  gcc/testsuite/gcc.target/riscv/tlsdesc.c | 12 ++++++++++++
>  11 files changed, 115 insertions(+), 11 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/tls_1.x
>  create mode 100644 gcc/testsuite/gcc.target/riscv/tlsdesc.c
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 17873ac2103..1a5870672d2 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -2492,6 +2492,7 @@ riscv*-*-linux*)
>         # Force .init_array support.  The configure script cannot always
>         # automatically detect that GAS supports it, yet we require it.
>         gcc_cv_initfini_array=yes
> +       with_tls=${with_tls:-trad}
>         ;;
>  riscv*-*-elf* | riscv*-*-rtems*)
>         tm_file="elfos.h newlib-stdint.h ${tm_file} riscv/elf.h"
> @@ -2534,6 +2535,7 @@ riscv*-*-freebsd*)
>         # Force .init_array support.  The configure script cannot always
>         # automatically detect that GAS supports it, yet we require it.
>         gcc_cv_initfini_array=yes
> +       with_tls=${with_tls:-trad}
>         ;;
>
>  loongarch*-*-linux*)
> @@ -4671,7 +4673,7 @@ case "${target}" in
>                 ;;
>
>         riscv*-*-*)
> -               supported_defaults="abi arch tune riscv_attribute isa_spec"
> +               supported_defaults="abi arch tune riscv_attribute isa_spec tls"
>
>                 case "${target}" in
>                 riscv-* | riscv32*) xlen=32 ;;
> @@ -4801,6 +4803,17 @@ case "${target}" in
>                                 ;;
>                         esac
>                 fi
> +               # Handle --with-tls.
> +               case "$with_tls" in
> +               "" \
> +               | trad | desc)
> +                       # OK
> +                       ;;
> +               *)
> +                       echo "Unknown TLS method used in --with-tls=$with_tls" 1>&2
> +                       exit 1
> +                       ;;
> +               esac
>
>                 # Handle --with-multilib-list.
>                 if test "x${with_multilib_list}" != xdefault; then
> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> index 392b9169240..1b2dd5757a8 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -154,4 +154,10 @@ enum rvv_vector_bits_enum {
>  #define TARGET_MAX_LMUL                                                        \
>    (int) (rvv_max_lmul == RVV_DYNAMIC ? RVV_M8 : rvv_max_lmul)
>
> +/* TLS types.  */
> +enum riscv_tls_type {
> +  TLS_TRADITIONAL,
> +  TLS_DESCRIPTORS
> +};
> +
>  #endif /* ! GCC_RISCV_OPTS_H */
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index b8735593805..066a5a5a7cc 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -34,9 +34,10 @@ enum riscv_symbol_type {
>    SYMBOL_TLS,
>    SYMBOL_TLS_LE,
>    SYMBOL_TLS_IE,
> -  SYMBOL_TLS_GD
> +  SYMBOL_TLS_GD,
> +  SYMBOL_TLSDESC,
>  };
> -#define NUM_SYMBOL_TYPES (SYMBOL_TLS_GD + 1)
> +#define NUM_SYMBOL_TYPES (SYMBOL_TLSDESC + 1)
>
>  /* Classifies an address.
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index fe9976bfffe..deac3e4e9e3 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -1072,6 +1072,7 @@ static int riscv_symbol_insns (enum riscv_symbol_type type)
>      case SYMBOL_ABSOLUTE: return 2; /* LUI + the reference.  */
>      case SYMBOL_PCREL: return 2; /* AUIPC + the reference.  */
>      case SYMBOL_TLS_LE: return 3; /* LUI + ADD TP + the reference.  */
> +    case SYMBOL_TLSDESC: return 6; /* 4-instruction call + ADD TP + the reference.  */
>      case SYMBOL_GOT_DISP: return 3; /* AUIPC + LD GOT + the reference.  */
>      case SYMBOL_FORCE_TO_MEM: return 3; /* AUIPC + LD + the reference.  */
>      default: gcc_unreachable ();
> @@ -2220,7 +2221,7 @@ riscv_call_tls_get_addr (rtx sym, rtx result)
>  static rtx
>  riscv_legitimize_tls_address (rtx loc)
>  {
> -  rtx dest, tp, tmp;
> +  rtx dest, tp, tmp, a0;
>    enum tls_model model = SYMBOL_REF_TLS_MODEL (loc);
>
>  #if 0
> @@ -2236,9 +2237,24 @@ riscv_legitimize_tls_address (rtx loc)
>        /* Rely on section anchors for the optimization that LDM TLS
>          provides.  The anchor's address is loaded with GD TLS. */
>      case TLS_MODEL_GLOBAL_DYNAMIC:
> -      tmp = gen_rtx_REG (Pmode, GP_RETURN);
> -      dest = gen_reg_rtx (Pmode);
> -      emit_libcall_block (riscv_call_tls_get_addr (loc, tmp), dest, tmp, loc);
> +      if (TARGET_TLSDESC)
> +       {
> +         static unsigned seqno;
> +         tp = gen_rtx_REG (Pmode, THREAD_POINTER_REGNUM);
> +         a0 = gen_rtx_REG (Pmode, GP_ARG_FIRST);
> +         dest = gen_reg_rtx (Pmode);
> +
> +         emit_insn (gen_tlsdesc (Pmode, loc, GEN_INT (seqno)));
> +         emit_insn (gen_add3_insn (dest, a0, tp));
> +         seqno++;
> +       }
> +      else
> +       {
> +         tmp = gen_rtx_REG (Pmode, GP_RETURN);
> +         dest = gen_reg_rtx (Pmode);
> +         emit_libcall_block (riscv_call_tls_get_addr (loc, tmp), dest, tmp,
> +                             loc);
> +       }
>        break;
>
>      case TLS_MODEL_INITIAL_EXEC:
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index da089a03e9d..10ad4aedd70 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -64,6 +64,7 @@ extern const char *riscv_arch_help (int argc, const char **argv);
>     --with-abi is ignored if -mabi is specified.
>     --with-tune is ignored if -mtune or -mcpu is specified.
>     --with-isa-spec is ignored if -misa-spec is specified.
> +   --with-tls is ignored if -mtls-dialect is specified.
>
>     But using default -march/-mtune value if -mcpu don't have valid option.  */
>  #define OPTION_DEFAULT_SPECS \
> @@ -73,8 +74,9 @@ extern const char *riscv_arch_help (int argc, const char **argv);
>    {"arch", "%{!march=*:"                                               \
>            "  %{!mcpu=*:-march=%(VALUE)}"                               \
>            "  %{mcpu=*:%:riscv_expand_arch_from_cpu(%* %(VALUE))}}" },  \
> -  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" }, \
> -  {"isa_spec", "%{!misa-spec=*:-misa-spec=%(VALUE)}" }, \
> +  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" },                               \
> +  {"isa_spec", "%{!misa-spec=*:-misa-spec=%(VALUE)}" },                        \
> +  {"tls", "%{!mtls-dialect=*:-mtls-dialect=%(VALUE)}"},                \
>
>  #ifdef IN_LIBGCC2
>  #undef TARGET_64BIT
> @@ -1228,4 +1230,7 @@ extern void riscv_remove_unneeded_save_restore_calls (void);
>  #define HAVE_POST_MODIFY_DISP TARGET_XTHEADMEMIDX
>  #define HAVE_PRE_MODIFY_DISP  TARGET_XTHEADMEMIDX
>
> +/* Check TLS Descriptors mechanism is selected.  */
> +#define TARGET_TLSDESC (riscv_tls_dialect == TLS_DESCRIPTORS)
> +
>  #endif /* ! GCC_RISCV_H */
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 0346cc3859d..c2b4323c53a 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -48,7 +48,7 @@
>    UNSPEC_TLS_LE
>    UNSPEC_TLS_IE
>    UNSPEC_TLS_GD
> -
> +  UNSPEC_TLSDESC
>    ;; High part of PC-relative address.
>    UNSPEC_AUIPC
>
> @@ -2089,6 +2089,24 @@
>     (set_attr "type" "load")
>     (set_attr "mode" "<MODE>")])
>
> +(define_insn "@tlsdesc<mode>"
> +  [(set (reg:P A0_REGNUM)
> +           (unspec:P
> +                       [(match_operand:P 0 "symbolic_operand" "")
> +                        (match_operand:P 1 "const_int_operand")]
> +                       UNSPEC_TLSDESC))
> +   (clobber (reg:P T0_REGNUM))]
> +  "TARGET_TLSDESC"
> +  {
> +    return ".LT%1: auipc\ta0,%%tlsdesc_hi(%0)\;"
> +           "<load>\tt0,%%tlsdesc_load_lo(.LT%1)(a0)\;"
> +           "addi\ta0,a0,%%tlsdesc_add_lo(.LT%1)\;"
> +           "jalr\tt0,t0,%%tlsdesc_call(.LT%1)";
> +  }
> +  [(set_attr "type" "multi")
> +   (set_attr "length" "16")
> +   (set_attr "mode" "<MODE>")])
> +
>  (define_insn "auipc<mode>"
>    [(set (match_operand:P           0 "register_operand" "=r")
>         (unspec:P
> diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
> index 7d625af02d5..8da0764eb4b 100644
> --- a/gcc/config/riscv/riscv.opt
> +++ b/gcc/config/riscv/riscv.opt
> @@ -606,3 +606,17 @@ Enum(rvv_vector_bits) String(zvl) Value(RVV_VECTOR_BITS_ZVL)
>  mrvv-vector-bits=
>  Target RejectNegative Joined Enum(rvv_vector_bits) Var(rvv_vector_bits) Init(RVV_VECTOR_BITS_SCALABLE)
>  -mrvv-vector-bits=<string>     Set the kind of bits for an RVV vector register.
> +
> +Enum
> +Name(tls_type) Type(enum riscv_tls_type)
> +The possible TLS dialects:
> +
> +EnumValue
> +Enum(tls_type) String(trad) Value(TLS_TRADITIONAL)
> +
> +EnumValue
> +Enum(tls_type) String(desc) Value(TLS_DESCRIPTORS)
> +
> +mtls-dialect=
> +Target RejectNegative Joined Enum(tls_type) Var(riscv_tls_dialect) Init(TLS_TRADITIONAL) Save
> +Specify TLS dialect.
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 269fe7ec870..120bb045f4d 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -1187,6 +1187,9 @@ Specify the default TLS dialect, for systems were there is a choice.
>  For ARM targets, possible values for @var{dialect} are @code{gnu} or
>  @code{gnu2}, which select between the original GNU dialect and the GNU TLS
>  descriptor-based dialect.
> +For RISC-V targets, possible values for @var{dialect} are @code{trad} or
> +@code{desc}, which select between the traditional GNU dialect and the GNU TLS
> +descriptor-based dialect.
>
>  @item --enable-multiarch
>  Specify whether to enable or disable multiarch support.  The default is
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index d09074e13de..e4112bbe61e 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1262,7 +1262,8 @@ See RS/6000 and PowerPC Options.
>  -minline-atomics  -mno-inline-atomics
>  -minline-strlen  -mno-inline-strlen
>  -minline-strcmp  -mno-inline-strcmp
> --minline-strncmp  -mno-inline-strncmp}
> +-minline-strncmp  -mno-inline-strncmp
> +-mtls-dialect=desc  -mtls-dialect=trad}
>
>  @emph{RL78 Options}
>  @gccoptlist{-msim  -mmul=none  -mmul=g13  -mmul=g14  -mallregs
> @@ -30975,6 +30976,16 @@ which register to use as base register for reading the canary,
>  and from what offset from that base register. There is no default
>  register or offset as this is entirely for use within the Linux
>  kernel.
> +
> +@opindex mtls-dialect=desc
> +@item -mtls-dialect=desc
> +Use TLS descriptors as the thread-local storage mechanism for dynamic accesses
> +of TLS variables.
> +
> +@opindex mtls-dialect=trad
> +@item -mtls-dialect=trad
> +Use traditional TLS as the thread-local storage mechanism for dynamic accesses
> +of TLS variables.  This is the default.
>  @end table
>
>  @node RL78 Options
> diff --git a/gcc/testsuite/gcc.target/riscv/tls_1.x b/gcc/testsuite/gcc.target/riscv/tls_1.x
> new file mode 100644
> index 00000000000..cf334ef55ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/tls_1.x
> @@ -0,0 +1,5 @@
> +extern __thread unsigned gd;
> +
> +unsigned get() {
> +       return gd;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/tlsdesc.c b/gcc/testsuite/gcc.target/riscv/tlsdesc.c
> new file mode 100644
> index 00000000000..f9ac9fe0d6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/tlsdesc.c
> @@ -0,0 +1,12 @@
> +/* { dg-require-effective-target tls_native } */
> +/* { dg-options "-O2 -fpic -mtls-dialect=desc --save-temps" } */
> +/* { dg-require-effective-target fpic } */
> +
> +#include "tls_1.x"
> +
> +/* { dg-final { scan-assembler-times "auipc\t\[a-x0-9\]+,%tlsdesc_hi" 1 } } */
> +/* { dg-final { scan-assembler-times "lw\t\[a-x0-9\]+,%tlsdesc_load_lo" 1 { target { rv32 } } } } */
> +/* { dg-final { scan-assembler-times "ld\t\[a-x0-9\]+,%tlsdesc_load_lo" 1 { target { rv64 } } } }*/
> +/* { dg-final { scan-assembler-times "addi\ta0,\[a-x0-9\]+,%tlsdesc_add_lo" 1 } } */
> +/* { dg-final { scan-assembler-times "jalr\tt0,\[a-x0-9\]+,%tlsdesc_call" 1 } } */
> +/* { dg-final { cleanup-saved-temps } } */
> --
> 2.44.0
>