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

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

Hi Tatsuyuki:

Thanks your TLS desc implementation, it's looks already in good shape
now! just few minor comment :)


> @@ -121,6 +121,14 @@
>     (T1_REGNUM                  6)
>     (S0_REGNUM                  8)
>     (S1_REGNUM                  9)
> +   (A0_REGNUM                  10)
> +   (A1_REGNUM                  11)
> +   (A2_REGNUM                  12)
> +   (A3_REGNUM                  13)
> +   (A4_REGNUM                  14)
> +   (A5_REGNUM                  15)
> +   (A6_REGNUM                  16)
> +   (A7_REGNUM                  17)

Drop A1_REGNUM~A7_REGNUM, they seems unused.

>     (S2_REGNUM                  18)
>     (S3_REGNUM                  19)
>     (S4_REGNUM                  20)
> @@ -1869,6 +1877,18 @@
>    [(set_attr "got" "load")
>     (set_attr "mode" "<MODE>")])
>
> +(define_insn "tlsdesc<mode>"

You can add @ to the pattern name ("@tlsdesc<mode>"), then you can
simplify riscv_legitimize_tls_address like that:

@@ -1908,7 +1898,7 @@ riscv_legitimize_tls_address (rtx loc)
         a0 = gen_rtx_REG (Pmode, GP_ARG_FIRST);
         dest = gen_reg_rtx (Pmode);

-         emit_insn (riscv_tlsdesc (loc, GEN_INT (seqno)));
+         emit_insn (gen_tlsdesc (Pmode, loc, GEN_INT (seqno)));
         emit_insn (gen_add3_insn (dest, a0, tp));
         seqno++;
       }

> +  [(set (reg:P A0_REGNUM)
> +           (unspec:P
> +                       [(match_operand:P 0 "symbolic_operand" "")
> +             (match_operand:P 1 "const_int_operand")]
> +                       UNSPEC_TLSDESC))
> +   (clobber (reg:SI T0_REGNUM))]
> +  "TARGET_TLSDESC"
> +  ".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)"

I would suggest using return rather than single long line, like below,
that would be easier to read :
{
 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")

We need add length here, something like that: "(set_attr "length"
(const_int 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 6304efebfd5..40b3ebf2a99 100644
> --- a/gcc/config/riscv/riscv.opt
> +++ b/gcc/config/riscv/riscv.opt
> @@ -311,3 +311,17 @@ Enum(riscv_autovec_lmul) String(m8) Value(RVV_M8)
>  -param=riscv-autovec-lmul=
>  Target RejectNegative Joined Enum(riscv_autovec_lmul) Var(riscv_autovec_lmul) Init(RVV_M1)
>  -param=riscv-autovec-lmul=<string>     Set the RVV LMUL of auto-vectorization in the RISC-V port.
> +
> +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

Could you add a configure option `--with-tls` to control the default
TLS dialect?

You can reference ARM:
gcc/gcc/config/arm/arm.h: grep "OPTION_DEFAULT_SPECS"
gcc/gcc/config.gcc: grep "with_tls"

> +Specify TLS dialect.
> \ No newline at end of file
> --
> 2.34.1
>