[PowerPC] Fix missing TLS symbol type.

[stefanp]

Summary

Previous implementations for the TLS models General Dynamic and Initial Exec
were missing the ELF::STT_TLS type on symbols that required the type. This patch
adds the type.

[MaskRay]

I'll not say this is a bug. For an undefined symbol, STT_TLS is not strictly required. For a definition, STT_TLS is indeed better.

[stefanp]

In D86777#2244533, @MaskRay wrote:

I'll not say this is a bug. For an undefined symbol, STT_TLS is not strictly required. For a definition, STT_TLS is indeed better.

MaskRay,
Thank you for looking at this patch.

Sorry it took me a couple of days to reply to this. I wanted to clarify in my head what was going on.
The reason I added this patch was because I ran into this following setup:

clang -O3 -mcpu=pwr10 -fPIC -c PCRelTLS.c

The source file PCRelTLS.c has some examples of General Dynamic and has two TLS variables defined as:

extern __thread unsigned int x;
extern __thread unsigned int y;

Now, if I look at the symbol table I end up with:

7: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND x
8: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND y

However, when I link this object:

ld.lld --shared PCRelTLS.o -o PCRelTLS.so

It looks like LLD will enter the function

template <class ELFT>
static unsigned
handleTlsRelocation(RelType type, Symbol &sym, InputSectionBase &c,
                    typename ELFT::uint offset, int64_t addend, RelExpr expr) {
  if (!sym.isTls())
    return 0;

and then exit immediately and the relocations related to x and y won't be handled correctly.
So, I did what you see in this patch: I made sure that all of the symbols like x and y had the type STT_TLS.

Of course, there could be something I've missed in my logic.

[MaskRay]

It is not well specified but I notice that GNU ld and/or gold support some TLS type relocations referencing STT_SECTION symbols. Supporting STT_SECTION can be difficult in LLD (https://bugs.llvm.org/show_bug.cgi?id=45600) and only provide minor size optimizations. Properly annotating symbols with STT_TLS is always nice - because STT_TLS symbols behave like in a different address space. An undefined STT_TLS can help with diagnostics.

[sfertile]

One minor nit, but otherwise LGTM.

llvm/lib/MC/MCELFStreamer.cpp

441

Real minor nit: Place this case with the other VK_PPC_GOT_TPREL cases.

[stefanp]

Fixed nit and moved the position of the case in the switch.

llvm/lib/MC/MCELFStreamer.cpp

case MCExpr::SymbolRef: {
     case MCSymbolRefExpr::VK_PPC_GOT_TPREL_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_TPREL_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_TPREL_HA:
     case MCSymbolRefExpr::VK_PPC_GOT_DTPREL:
     case MCSymbolRefExpr::VK_PPC_GOT_DTPREL_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_DTPREL_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_DTPREL_HA:
     case MCSymbolRefExpr::VK_PPC_TLS:
+    case MCSymbolRefExpr::VK_PPC_GOT_TPREL_PCREL:
+    case MCSymbolRefExpr::VK_PPC_TLS_PCREL:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_HA:
     case MCSymbolRefExpr::VK_PPC_TLSGD:
+    case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_PCREL:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_HA:
     case MCSymbolRefExpr::VK_PPC_TLSLD:
       break;
     }
     getAssembler().registerSymbol(symRef.getSymbol());

llvm/test/CodeGen/PowerPC/pcrel-tls-general-dynamic.ll

 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   --relocation-model=pic -mcpu=pwr10 -ppc-asm-full-reg-names \
 ; RUN:   -enable-ppc-pcrel-tls < %s | FileCheck %s --check-prefix=CHECK-S
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   --relocation-model=pic -mcpu=pwr10 -ppc-asm-full-reg-names \
-; RUN:   -enable-ppc-pcrel-tls --filetype=obj < %s | \
-; RUN:   llvm-objdump --mcpu=pwr10 -dr - | FileCheck %s --check-prefix=CHECK-O
+; RUN:   -enable-ppc-pcrel-tls --filetype=obj -o %t.o < %s
+; RUN: llvm-objdump --mcpu=pwr10 -dr %t.o |FileCheck %s --check-prefix=CHECK-O
+; RUN: llvm-readelf -s %t.o | FileCheck %s --check-prefix=CHECK-SYM
 
 ; These test cases are to ensure that when using pc relative memory operations
 ; ABI correct code and relocations are produced for General Dynamic TLS Model.
 
 @x = external thread_local global i32, align 4
 
 define nonnull i32* @GeneralDynamicAddressLoad() {
   ; CHECK-S-LABEL: GeneralDynamicAddressLoad:
define i32 @GeneralDynamicValueLoad() {
   ; CHECK-S-NEXT:    blr
   ; CHECK-O-LABEL: <GeneralDynamicValueLoad>:
   ; CHECK-O:         4c: 00 00 10 06 00 00 60 38       paddi 3, 0, 0, 1
   ; CHECK-O-NEXT:    000000000000004c:  R_PPC64_GOT_TLSGD_PCREL34    x
   ; CHECK-O-NEXT:    54: 01 00 00 48                   bl 0x54
   ; CHECK-O-NEXT:    0000000000000054:  R_PPC64_TLSGD        x
   ; CHECK-O-NEXT:    0000000000000054:  R_PPC64_REL24_NOTOC  __tls_get_addr
   ; CHECK-O-NEXT:    58: 00 00 63 80                   lwz 3, 0(3)
+
+  ; CHECK-SYM-LABEL: Symbol table '.symtab' contains 7 entries
+  ; CHECK-SYM:         6: 0000000000000000     0 TLS     GLOBAL DEFAULT  UND x
   entry:
     %0 = load i32, i32* @x, align 4
     ret i32 %0
 }

llvm/test/CodeGen/PowerPC/pcrel-tls-initial-exec.ll

 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -enable-ppc-pcrel-tls < %s | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-S
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names --filetype=obj \
-; RUN:   -enable-ppc-pcrel-tls < %s | llvm-objdump --mcpu=pwr10 -dr - | \
-; RUN:   FileCheck %s --check-prefix=CHECK-O
+; RUN:   -enable-ppc-pcrel-tls -o %t.o < %s
+; RUN: llvm-objdump --mcpu=pwr10 -dr %t.o | FileCheck %s --check-prefix=CHECK-O
+; RUN: llvm-readelf -s %t.o | FileCheck %s --check-prefix=CHECK-SYM
 
 ; These test cases are to ensure that when using pc relative memory operations
 ; ABI correct code and relocations are produced for Initial Exec TLS Model.
 ; Note that with R_PPC64_TLS relocations, to distinguish PC relative
 ; TLS the relocation has a field value displaced by one byte from the
 ; beginning of the instruction.
 
 @x = external thread_local global i32, align 4

 ; CHECK-S-NEXT:    lwzx r3, r3, x@tls@pcrel
 ; CHECK-S-NEXT:    blr
 ; CHECK-O-LABEL: <InitialExecValueLoad>:
 ; CHECK-O:         00 00 10 04 00 00 60 e4        pld 3, 0(0), 1
 ; CHECK-O-NEXT:    0000000000000020:  R_PPC64_GOT_TPREL_PCREL34 x
 ; CHECK-O-NEXT:    2e 68 63 7c                    lwzx 3, 3, 13
 ; CHECK-O-NEXT:    0000000000000029:  R_PPC64_TLS x
 ; CHECK-O-NEXT:    20 00 80 4e                    blr
+
+; CHECK-SYM-LABEL: Symbol table '.symtab' contains 6 entries
+; CHECK-SYM:         5: 0000000000000000     0 TLS     GLOBAL DEFAULT  UND x
 entry:
   %0 = load i32, i32* @x, align 4
   ret i32 %0
 }