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

[Tatsuyuki Ishi <ishitatsuyuki@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4332 bytes --]

> On Nov 16, 2023, at 10:07, Jeff Law <jeffreyalaw@gmail.com> wrote:
> 
> 
> 
> On 9/8/23 04:49, Tatsuyuki Ishi via Gcc-patches wrote:
>> This implements TLS Descriptors (TLSDESC) as specified in [1].
>> In TLSDESC instruction sequence, the first instruction relocates against
>> the target TLS variable, while subsequent instructions relocates against
>> the address of the first. Such usage of labels are not well-supported
>> within GCC. Due to this, the 4-instruction sequence is implemented as a
>> single RTX insn.
>> 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.
>> ---
> 
>> @@ -4694,6 +4696,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
> Is there a reason why this isn't formatted like the other cases?

Sorry, this was an oversight. I’ll fix it in the next version.

> 
>> @@ -1869,6 +1870,24 @@
>>    [(set_attr "got" "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:SI 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>")])
> Hmm, I would be a bit worried about explicitly using $a0 here.  That's generally frowned upon, but probably unavoidable in this case since this is a call under the hood.

Based on what I have read in the AArch64 backend, there are two ways to do this: introduce a custom calling convention, or put in a RTX insn that covers the whole sequence. Ideally we should do the first, but then there’s the label issue and it’s quite a bit more complicated. So I’m sticking with this for now.

> This needs changes to invoke.texi since it introduces new options.  I don't think it has to be anything terribly verbose.  A one liner is probably sufficient and I wouldn't be surprised if other ports have suitable text we could copy.

Ack.

> So overall if Kito's OK, then I am with the trivial doc change and perhaps the formatting fix in config.guess.

Sorry for all the delay on this. My progress has been (and still) blocked on supporting relaxation of TLSDESC in binutils (turns out you can’t run static binaries without relaxing it first). But that doesn’t seem exactly easy to do either, because relaxation that involves GOT elimination isn’t something we have in the RISC-V backend.

I’ll try to send a new version of this patch and get this unblocked on GCC side first.
Presumably this still needs the associated gas and ld support in place, so let me know if you want to merge this soon. I will ask on binutils for whether they could accept the basic part of the implementation without relaxation first.

Thanks,
Tatsuyuki. 

> jeff