[RISCV] Add support for TLSDESC

[quic-akaryaki]

No description provided.

[quic-akaryaki]

Looks like the build failed because upstream LLVM does not emit R_RISCV_ALIGN.

[quic-seaswara]

@lenary please review

[quic-seaswara]

I would like to see tests using 'C' code as well. You can still keep the asm test.

I would like to see more detailed testing mentioned below :

• Basic TLSDESC tests without relaxation (Executable, Shared library, PIE, static linking, symbol visibility tests)
  • Probably we want to have a flag --no-tlsdesc-relax
• TLSDESC tests with relaxation for the cases above
• Please add what relaxations is supported right now, and what is punted for later
  • It is not clear from the commit message on what TLS relaxationare included in this patch
• Any addition to map file for TLS relaxations will be useful
• Test cases with mno-relax as needed
• Documentation should follow as a seperate subtask (to know what the linker did for our users)

Please create subtasks as needed and totally I am ok to see the limited support that we add for 21.0 but my focus is to build this feature with more clarity.

cat > 1.c << \!
__attribute__((visibility("hidden"))) __thread int tls_var = 10;
int foo() { return tls_var; }
!

cat > 2.c << \!
extern __thread int tls_var;
int bar() { return tls_var; }
!

clang -c -target riscv32 1.c -fPIC -mtls-dialect=desc
clang -c -target riscv32 2.c -fPIC
# case 1
ld.eld -march riscv32 1.o 2.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE
# case 2 (partial link)
ld.eld -march riscv32 1.o 2.o -r -o r.o
ld.eld -march riscv32 r.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE
# case 3 (no relax)
clang -c -target riscv32 1.c -fPIC -mtls-dialect=desc -mno-relax
clang -c -target riscv32 2.c -fPIC -mno-relax
ld.eld -march riscv32 1.o 2.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE

[quic-akaryaki]

I would like to see tests using 'C' code as well. You can still keep the asm test.

I would like to see more detailed testing mentioned below :

• Basic TLSDESC tests without relaxation (Executable, Shared library, PIE, static linking, symbol visibility tests)

  • Probably we want to have a flag --no-tlsdesc-relax

• TLSDESC tests with relaxation for the cases above

• Please add what relaxations is supported right now, and what is punted for later

  • It is not clear from the commit message on what TLS relaxationare included in this patch

• Any addition to map file for TLS relaxations will be useful

• Test cases with mno-relax as needed

• Documentation should follow as a seperate subtask (to know what the linker did for our users)

Please create subtasks as needed and totally I am ok to see the limited support that we add for 21.0 but my focus is to build this feature with more clarity.

cat > 1.c << \!
__attribute__((visibility("hidden"))) __thread int tls_var = 10;
int foo() { return tls_var; }
!

cat > 2.c << \!
extern __thread int tls_var;
int bar() { return tls_var; }
!

clang -c -target riscv32 1.c -fPIC -mtls-dialect=desc
clang -c -target riscv32 2.c -fPIC
# case 1
ld.eld -march riscv32 1.o 2.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE
# case 2 (partial link)
ld.eld -march riscv32 1.o 2.o -r -o r.o
ld.eld -march riscv32 r.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE
# case 3 (no relax)
clang -c -target riscv32 1.c -fPIC -mtls-dialect=desc -mno-relax
clang -c -target riscv32 2.c -fPIC -mno-relax
ld.eld -march riscv32 1.o 2.o -shared -o 1.so
llvm-readelf --dyn-syms 1.so | grep tls_var && echo BADIMAGE
llvm-readelf -r 1.so | grep tls_var && echo BADIMAGE

Just to clarify, this patch has full support for TLSDESC, including all 4 types of static relocations, linker optimizations (replacing instructions with nops) and relaxations (removing instructions). Of course, it may have bugs, which is the case with existing code, which could be fixed before the 21 releases. Since new requirements come 1 day before the 21.x feature deadline, it won't go into the release.

Also, please clarify:
-- if --no-tlsdesc-relax is needed
-- which addition to map file for TLS relaxations will be useful

Thank you

[quic-seaswara]

Just to clarify, this patch has full support for TLSDESC, including all 4 types of static relocations, linker optimizations (replacing instructions with nops) and relaxations (removing instructions). Of course, it may have bugs, which is the case with existing code, which could be fixed before the 21 releases. Since new requirements come 1 day before the 21.x feature deadline, it won't go into the release.

Yes, sorry!.

My point was to make TLSDESC without relaxation work before relaxations are added.

Also cases such as TLSDESC with symbol visibility is something users typically do. We should look into those cases to make sure that we have tests that show functionality.

Also, please clarify: -- if --no-tlsdesc-relax is needed -- which addition to map file for TLS relaxations will be useful

I think it will be useful to have --no-tlsdesc-relax like --no-gp-relax or --no-relax-c to have a workaround if any.

Map file can be annotated to point out what relaxations were applied.

[quic-seaswara]

Great!. Let us get it merged.

@lenary if you can go through it and have your comments, it would be great!

[lenary]

I only have one high-level comment. Otherwise I'm happy.

[quic-akaryaki]

The last force push is only a rename no_riscv_tlsdesc -> no_relax_tlsdesc.