On Mon, Dec 4, 2023 at 11:02 PM Tatsuyuki Ishi <ishitatsuy...@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.
typo: --with-tls={trad,desc}.
The patch looks good to me, but I am not an approver:) I only have
some nitpicky comments.
Reviewed-by: Fangrui Song <mask...@google.com>
I know that the binutils patch needs some work, but I think it doesn't
have to block this patch.
BTW, Clang got -mtls-dialect=desc today, so one can test GCC by gcc
-S -fpic -mtls-dialect + clang -c + ld.lld + musl
https://maskray.me/blog/2024-01-23-riscv-tlsdesc-works
I actually tested my ld.lld patch by compiling with GCC and manually
replacing TLS GD code sequence with TLSDESC :)
> [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 rv64gc, 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.
>
> 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 748430194f3..8bb22e9f590 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -2490,6 +2490,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"
> @@ -2532,6 +2533,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*)
> @@ -4658,7 +4660,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 ;;
> @@ -4788,6 +4790,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 30efebbf07b..b2551968be0 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -138,4 +138,10 @@ enum stringop_strategy_enum {
> #define TARGET_MAX_LMUL
> \
> (int) (riscv_autovec_lmul == RVV_DYNAMIC ? RVV_M8 : riscv_autovec_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 695ee24ad6f..f0d8f88dad7 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -33,9 +33,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 3f111fa0393..639a77922d9 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -892,6 +892,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. */
> default: gcc_unreachable ();
> }
> @@ -1996,7 +1997,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
> @@ -2012,9 +2013,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 6df9ec73c5e..5eb08c21cf4 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -62,6 +62,7 @@ extern const char *riscv_multi_lib_check (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 \
> @@ -71,8 +72,9 @@ extern const char *riscv_multi_lib_check (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
> @@ -1215,4 +1217,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 a98918dfd43..c3676d751df 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -47,7 +47,7 @@
> UNSPEC_TLS_LE
> UNSPEC_TLS_IE
> UNSPEC_TLS_GD
> -
> + UNSPEC_TLSDESC
> ;; High part of PC-relative address.
> UNSPEC_AUIPC
>
> @@ -2017,6 +2017,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 59ce7106ecf..8de2e8628d8 100644
> --- a/gcc/config/riscv/riscv.opt
> +++ b/gcc/config/riscv/riscv.opt
> @@ -556,3 +556,17 @@ Enum(stringop_strategy) String(vector)
> Value(STRATEGY_VECTOR)
> mstringop-strategy=
> Target RejectNegative Joined Enum(stringop_strategy) Var(stringop_strategy)
> Init(STRATEGY_AUTO)
> Specify stringop expansion strategy.
> +
> +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 c1128d9274c..3b549ab62da 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -1184,6 +1184,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 681e3f3f466..62c8bf9061b 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1252,7 +1252,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
> @@ -29988,6 +29989,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.
> +
This can be simplified to:
"Use TLS descriptors for dynamic accesses of TLS variables."
I know aarch64 uses the term "mechanism" but "dialect" seems preferred?
I think in several places, "mechanism" can be changed to "dialect".
> +@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
Use the traditional general-dynamic TLS model for dynamic accesses of
TLS variables.
It's a pity that aarch64 doesn't have a -mtls-dialect=trad test that
actually checks assembly:(
> @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.43.0
Thanks for the test.
--
宋方睿
