Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PowerPC] Tune AIX shared library TLS model at function level #84132

Merged
merged 10 commits into from
May 9, 2024

Conversation

orcguru
Copy link
Contributor

@orcguru orcguru commented Mar 6, 2024

Under some circumstance (library loaded with the main program), TLS initial-exec and local-dynamic access model can be interchanged. We could use some simple heuristic to decide use which TLS model when the variable is accessed in which function:

  • If there is single TLS variable access in the function, use TLS initial-exec model.
  • If there are multiple TLS variable accesses in the function, use TLS local-dynamic model.

@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2024

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-powerpc

Author: Felix (Ting Wang) (orcguru)

Changes

On AIX, rename those tls-local-dynamic referenced TOC symbols, so that in following patches we can switch between different TLS models (local-dynamic or initial-exec) for the same TLS GV in different functions within the same module.


Patch is 34.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84132.diff

13 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h (+3-2)
  • (modified) llvm/include/llvm/MC/MCContext.h (+2-1)
  • (modified) llvm/include/llvm/Target/TargetLoweringObjectFile.h (+4-2)
  • (modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (+6-2)
  • (modified) llvm/lib/MC/MCContext.cpp (+3-2)
  • (modified) llvm/lib/MC/MCSymbolXCOFF.cpp (+5-1)
  • (modified) llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp (+14-1)
  • (modified) llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp (+3-3)
  • (modified) llvm/test/CodeGen/PowerPC/aix-tls-gd-double.ll (+8-4)
  • (modified) llvm/test/CodeGen/PowerPC/aix-tls-gd-int.ll (+8-4)
  • (modified) llvm/test/CodeGen/PowerPC/aix-tls-gd-longlong.ll (+16-8)
  • (added) llvm/test/CodeGen/PowerPC/aix-tls-ld-unqualified-symbols.ll (+362)
  • (modified) llvm/test/CodeGen/PowerPC/aix-tls-local-dynamic.ll (+26-13)
diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index 8eef45ce565deb..1aa25e98423afa 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -279,8 +279,9 @@ class TargetLoweringObjectFileXCOFF : public TargetLoweringObjectFile {
   MCSection *
   getSectionForFunctionDescriptor(const Function *F,
                                   const TargetMachine &TM) const override;
-  MCSection *getSectionForTOCEntry(const MCSymbol *Sym,
-                                   const TargetMachine &TM) const override;
+  MCSection *
+  getSectionForTOCEntry(const MCSymbol *Sym, const TargetMachine &TM,
+                        const MCSymbolRefExpr::VariantKind VK) const override;
 
   /// For external functions, this will always return a function descriptor
   /// csect.
diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h
index 68d6f3e59d2d41..5e1473b7f78eb8 100644
--- a/llvm/include/llvm/MC/MCContext.h
+++ b/llvm/include/llvm/MC/MCContext.h
@@ -678,7 +678,8 @@ class MCContext {
       std::optional<XCOFF::CsectProperties> CsectProp = std::nullopt,
       bool MultiSymbolsAllowed = false, const char *BeginSymName = nullptr,
       std::optional<XCOFF::DwarfSectionSubtypeFlags> DwarfSubtypeFlags =
-          std::nullopt);
+          std::nullopt,
+      StringRef RenamePrefix = StringRef());
 
   // Create and save a copy of STI and return a reference to the copy.
   MCSubtargetInfo &getSubtargetCopy(const MCSubtargetInfo &STI);
diff --git a/llvm/include/llvm/Target/TargetLoweringObjectFile.h b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
index 0c09cfe684783b..31a466a18d77a2 100644
--- a/llvm/include/llvm/Target/TargetLoweringObjectFile.h
+++ b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
 #define LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
 
+#include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCRegister.h"
 #include <cstdint>
@@ -256,8 +257,9 @@ class TargetLoweringObjectFile : public MCObjectFileInfo {
   /// On targets that support TOC entries, return a section for the entry given
   /// the symbol it refers to.
   /// TODO: Implement this interface for existing ELF targets.
-  virtual MCSection *getSectionForTOCEntry(const MCSymbol *S,
-                                           const TargetMachine &TM) const {
+  virtual MCSection *
+  getSectionForTOCEntry(const MCSymbol *S, const TargetMachine &TM,
+                        const MCSymbolRefExpr::VariantKind VK) const {
     return nullptr;
   }
 
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 6943ce261d9d9c..ec17ceb9c83ece 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -2679,7 +2679,8 @@ MCSection *TargetLoweringObjectFileXCOFF::getSectionForFunctionDescriptor(
 }
 
 MCSection *TargetLoweringObjectFileXCOFF::getSectionForTOCEntry(
-    const MCSymbol *Sym, const TargetMachine &TM) const {
+    const MCSymbol *Sym, const TargetMachine &TM,
+    const MCSymbolRefExpr::VariantKind VK) const {
   // Use TE storage-mapping class when large code model is enabled so that
   // the chance of needing -bbigtoc is decreased. Also, the toc-entry for
   // EH info is never referenced directly using instructions so it can be
@@ -2694,7 +2695,10 @@ MCSection *TargetLoweringObjectFileXCOFF::getSectionForTOCEntry(
            cast<MCSymbolXCOFF>(Sym)->isEHInfo())
               ? XCOFF::XMC_TE
               : XCOFF::XMC_TC,
-          XCOFF::XTY_SD));
+          XCOFF::XTY_SD),
+      /*MultiSymbolsAllowed=*/false, /*BeginSymName=*/nullptr,
+      /*DwarfSubtypeFlags=*/std::nullopt,
+      (VK == MCSymbolRefExpr::VK_PPC_AIX_TLSLD ? "_$TLSLD." : StringRef()));
 }
 
 MCSection *TargetLoweringObjectFileXCOFF::getSectionForLSDA(
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index ba5cefaf18c1fd..dd289147034371 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -780,7 +780,8 @@ MCSectionXCOFF *MCContext::getXCOFFSection(
     StringRef Section, SectionKind Kind,
     std::optional<XCOFF::CsectProperties> CsectProp, bool MultiSymbolsAllowed,
     const char *BeginSymName,
-    std::optional<XCOFF::DwarfSectionSubtypeFlags> DwarfSectionSubtypeFlags) {
+    std::optional<XCOFF::DwarfSectionSubtypeFlags> DwarfSectionSubtypeFlags,
+    StringRef RenamePrefix) {
   bool IsDwarfSec = DwarfSectionSubtypeFlags.has_value();
   assert((IsDwarfSec != CsectProp.has_value()) && "Invalid XCOFF section!");
 
@@ -806,7 +807,7 @@ MCSectionXCOFF *MCContext::getXCOFFSection(
     QualName = cast<MCSymbolXCOFF>(getOrCreateSymbol(CachedName));
   else
     QualName = cast<MCSymbolXCOFF>(getOrCreateSymbol(
-        CachedName + "[" +
+        RenamePrefix + CachedName + "[" +
         XCOFF::getMappingClassString(CsectProp->MappingClass) + "]"));
 
   MCSymbol *Begin = nullptr;
diff --git a/llvm/lib/MC/MCSymbolXCOFF.cpp b/llvm/lib/MC/MCSymbolXCOFF.cpp
index b4c96a1ffa2333..17f0bcf77745af 100644
--- a/llvm/lib/MC/MCSymbolXCOFF.cpp
+++ b/llvm/lib/MC/MCSymbolXCOFF.cpp
@@ -24,7 +24,11 @@ void MCSymbolXCOFF::setRepresentedCsect(MCSectionXCOFF *C) {
   assert((!RepresentedCsect || RepresentedCsect == C) &&
          "Trying to set a csect that doesn't match the one that this symbol is "
          "already mapped to.");
-  assert(getSymbolTableName().equals(C->getSymbolTableName()) &&
+  // Csect representation related symbols access by using TLS local-dynamic
+  // model have prefix "_$TLSLD." before their names.
+  assert((getSymbolTableName().equals(C->getSymbolTableName()) ||
+          getSymbolTableName().equals(std::string("_$TLSLD.") +
+                                      C->getSymbolTableName().str())) &&
          "SymbolTableNames need to be the same for this symbol and its csect "
          "representation.");
   RepresentedCsect = C;
diff --git a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
index b849b7be7b7be8..0263c3588a2283 100644
--- a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
+++ b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
@@ -231,6 +231,20 @@ class PPCTargetAsmStreamer : public PPCTargetStreamer {
       MCSymbolXCOFF *TCSym =
           cast<MCSectionXCOFF>(Streamer.getCurrentSectionOnly())
               ->getQualNameSymbol();
+
+      // On AIX, symbol accessed using TLS local-dynamic model has been renamed
+      // by adding prefix "_$TLSLD.". Do assert that it has been renamed, and
+      // then emit the .rename with the original symbol name.
+      if (Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSLD) {
+        assert(TCSym->hasRename());
+        OS << "\t.tc " << TCSym->getName() << "," << XSym->getName() << "@"
+           << MCSymbolRefExpr::getVariantKindName(Kind) << '\n';
+        StringRef Lhs, Rhs;
+        std::tie(Lhs, Rhs) = TCSym->getSymbolTableName().split("_$TLSLD.");
+        Streamer.emitXCOFFRenameDirective(TCSym, Rhs);
+        return;
+      }
+
       // On AIX, we have TLS variable offsets (symbol@({gd|ie|le|ld}) depending
       // on the TLS access method (or model). For the general-dynamic access
       // method, we also have region handle (symbol@m) for each variable. For
@@ -242,7 +256,6 @@ class PPCTargetAsmStreamer : public PPCTargetStreamer {
           Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSGDM ||
           Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSIE ||
           Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSLE ||
-          Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSLD ||
           Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSML)
         OS << "\t.tc " << TCSym->getName() << "," << XSym->getName() << "@"
            << MCSymbolRefExpr::getVariantKindName(Kind) << '\n';
diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
index 9396ca22dacf86..4bf61a74ea7693 100644
--- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -2872,10 +2872,10 @@ void PPCAIXAsmPrinter::emitEndOfAsmFile(Module &M) {
       Name += cast<MCSymbolXCOFF>(I.first.first)->getSymbolTableName();
       MCSymbol *S = OutContext.getOrCreateSymbol(Name);
       TCEntry = cast<MCSectionXCOFF>(
-          getObjFileLowering().getSectionForTOCEntry(S, TM));
+          getObjFileLowering().getSectionForTOCEntry(S, TM, I.first.second));
     } else {
-      TCEntry = cast<MCSectionXCOFF>(
-          getObjFileLowering().getSectionForTOCEntry(I.first.first, TM));
+      TCEntry = cast<MCSectionXCOFF>(getObjFileLowering().getSectionForTOCEntry(
+          I.first.first, TM, I.first.second));
     }
     OutStreamer->switchSection(TCEntry);
 
diff --git a/llvm/test/CodeGen/PowerPC/aix-tls-gd-double.ll b/llvm/test/CodeGen/PowerPC/aix-tls-gd-double.ll
index ae41b6b1301064..d84f92b11c9781 100644
--- a/llvm/test/CodeGen/PowerPC/aix-tls-gd-double.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-tls-gd-double.ll
@@ -636,7 +636,8 @@ entry:
 ; SMALL32-NEXT:   .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL32-NEXT:   .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL32-LABEL:  L..C5:
-; SMALL32-NEXT:   .tc TIInit[TC],TIInit[TL]@ld
+; SMALL32-NEXT:   .tc _Renamed..5f24__TLSLD.TIInit[TC],TIInit[TL]@ld
+; SMALL32-NEXT:   .rename _Renamed..5f24__TLSLD.TIInit[TC],"TIInit"
 ; SMALL32-LABEL:  L..C6:
 ; SMALL32-NEXT:   .tc .TWInit[TC],TWInit[TL]@m
 ; SMALL32-LABEL:  L..C7:
@@ -654,7 +655,8 @@ entry:
 ; LARGE32-LABEL:  L..C3:
 ; LARGE32-NEXT:   .tc TGInit[TE],TGInit[TL]@gd
 ; LARGE32-LABEL:  L..C4:
-; LARGE32-NEXT:   .tc TIInit[TE],TIInit[TL]@ld
+; LARGE32-NEXT:   .tc _Renamed..5f24__TLSLD.TIInit[TE],TIInit[TL]@ld
+; LARGE32-NEXT:   .rename _Renamed..5f24__TLSLD.TIInit[TE],"TIInit"
 ; LARGE32-LABEL:  L..C5:
 ; LARGE32-NEXT:   .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE32-NEXT:   .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
@@ -678,7 +680,8 @@ entry:
 ; SMALL64-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL64-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL64-LABEL:  L..C5:
-; SMALL64-NEXT:  .tc TIInit[TC],TIInit[TL]@ld
+; SMALL64-NEXT:   .tc _Renamed..5f24__TLSLD.TIInit[TC],TIInit[TL]@ld
+; SMALL64-NEXT:   .rename _Renamed..5f24__TLSLD.TIInit[TC],"TIInit"
 ; SMALL64-LABEL:  L..C6:
 ; SMALL64-NEXT:  .tc .TWInit[TC],TWInit[TL]@m
 ; SMALL64-LABEL:  L..C7:
@@ -699,7 +702,8 @@ entry:
 ; LARGE64-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE64-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; LARGE64-LABEL:  L..C5:
-; LARGE64-NEXT:  .tc TIInit[TE],TIInit[TL]@ld
+; LARGE64-NEXT:   .tc _Renamed..5f24__TLSLD.TIInit[TE],TIInit[TL]@ld
+; LARGE64-NEXT:   .rename _Renamed..5f24__TLSLD.TIInit[TE],"TIInit"
 ; LARGE64-LABEL:  L..C6:
 ; LARGE64-NEXT:  .tc .TWInit[TE],TWInit[TL]@m
 ; LARGE64-LABEL:  L..C7:
diff --git a/llvm/test/CodeGen/PowerPC/aix-tls-gd-int.ll b/llvm/test/CodeGen/PowerPC/aix-tls-gd-int.ll
index bbb8e04b67b95e..f9567840a19651 100644
--- a/llvm/test/CodeGen/PowerPC/aix-tls-gd-int.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-tls-gd-int.ll
@@ -651,7 +651,8 @@ entry:
 ; SMALL32-NEXT:	 .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL32-NEXT:	 .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL32-LABEL: L..C5:
-; SMALL32-NEXT:	 .tc TIUninit[TC],TIUninit[UL]@ld
+; SMALL32-NEXT:	 .tc _Renamed..5f24__TLSLD.TIUninit[TC],TIUninit[UL]@ld
+; SMALL32-NEXT:	 .rename _Renamed..5f24__TLSLD.TIUninit[TC],"TIUninit"
 ; SMALL32-LABEL: L..C6:
 ; SMALL32-NEXT:	 .tc .TWUninit[TC],TWUninit[TL]@m
 ; SMALL32-LABEL: L..C7:
@@ -669,7 +670,8 @@ entry:
 ; LARGE32-LABEL: L..C3:
 ; LARGE32-NEXT:  .tc TGInit[TE],TGInit[TL]@gd
 ; LARGE32-LABEL: L..C4:
-; LARGE32-NEXT:  .tc TIUninit[TE],TIUninit[UL]@ld
+; LARGE32-NEXT:	 .tc _Renamed..5f24__TLSLD.TIUninit[TE],TIUninit[UL]@ld
+; LARGE32-NEXT:	 .rename _Renamed..5f24__TLSLD.TIUninit[TE],"TIUninit"
 ; LARGE32-LABEL: L..C5:
 ; LARGE32-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE32-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
@@ -693,7 +695,8 @@ entry:
 ; SMALL64-NEXT:   .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL64-NEXT:   .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL64-LABEL:  L..C5:
-; SMALL64-NEXT:   .tc TIUninit[TC],TIUninit[UL]@ld
+; SMALL64-NEXT:	 .tc _Renamed..5f24__TLSLD.TIUninit[TC],TIUninit[UL]@ld
+; SMALL64-NEXT:	 .rename _Renamed..5f24__TLSLD.TIUninit[TC],"TIUninit"
 ; SMALL64-LABEL:  L..C6:
 ; SMALL64-NEXT:   .tc .TWUninit[TC],TWUninit[TL]@m
 ; SMALL64-LABEL:  L..C7:
@@ -714,7 +717,8 @@ entry:
 ; LARGE64-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE64-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; LARGE64-LABEL:  L..C5:
-; LARGE64-NEXT:  .tc TIUninit[TE],TIUninit[UL]@ld
+; LARGE64-NEXT:	 .tc _Renamed..5f24__TLSLD.TIUninit[TE],TIUninit[UL]@ld
+; LARGE64-NEXT:	 .rename _Renamed..5f24__TLSLD.TIUninit[TE],"TIUninit"
 ; LARGE64-LABEL:  L..C6:
 ; LARGE64-NEXT:  .tc .TWUninit[TE],TWUninit[TL]@m
 ; LARGE64-LABEL:  L..C7:
diff --git a/llvm/test/CodeGen/PowerPC/aix-tls-gd-longlong.ll b/llvm/test/CodeGen/PowerPC/aix-tls-gd-longlong.ll
index ff087a2144488c..06937635bbd183 100644
--- a/llvm/test/CodeGen/PowerPC/aix-tls-gd-longlong.ll
+++ b/llvm/test/CodeGen/PowerPC/aix-tls-gd-longlong.ll
@@ -687,9 +687,11 @@ entry:
 ; SMALL32-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL32-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL32-LABEL:  L..C3:
-; SMALL32-NEXT:  .tc TIUninit[TC],TIUninit[UL]@ld
+; SMALL32-NEXT:  .tc _Renamed..5f24__TLSLD.TIUninit[TC],TIUninit[UL]@ld
+; SMALL32-NEXT:  .rename _Renamed..5f24__TLSLD.TIUninit[TC],"TIUninit"
 ; SMALL32-LABEL:  L..C4:
-; SMALL32-NEXT:  .tc TIInit[TC],TIInit[TL]@ld
+; SMALL32-NEXT:  .tc _Renamed..5f24__TLSLD.TIInit[TC],TIInit[TL]@ld
+; SMALL32-NEXT:  .rename _Renamed..5f24__TLSLD.TIInit[TC],"TIInit"
 ; SMALL32-LABEL:  L..C5:
 ; SMALL32-NEXT:  .tc .TWInit[TC],TWInit[TL]@m
 ; SMALL32-LABEL:  L..C6:
@@ -703,12 +705,14 @@ entry:
 ; LARGE32-LABEL:  L..C1:
 ; LARGE32-NEXT:  .tc TGInit[TE],TGInit[TL]@gd
 ; LARGE32-LABEL:  L..C2:
-; LARGE32-NEXT:  .tc TIUninit[TE],TIUninit[UL]@ld
+; LARGE32-NEXT:  .tc _Renamed..5f24__TLSLD.TIUninit[TE],TIUninit[UL]@ld
+; LARGE32-NEXT:  .rename _Renamed..5f24__TLSLD.TIUninit[TE],"TIUninit"
 ; LARGE32-LABEL:  L..C3:
 ; LARGE32-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE32-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; LARGE32-LABEL:  L..C4:
-; LARGE32-NEXT:  .tc TIInit[TE],TIInit[TL]@ld
+; LARGE32-NEXT:  .tc _Renamed..5f24__TLSLD.TIInit[TE],TIInit[TL]@ld
+; LARGE32-NEXT:  .rename _Renamed..5f24__TLSLD.TIInit[TE],"TIInit"
 ; LARGE32-LABEL:  L..C5:
 ; LARGE32-NEXT:  .tc .TWInit[TE],TWInit[TL]@m
 ; LARGE32-LABEL:  L..C6:
@@ -725,9 +729,11 @@ entry:
 ; SMALL64-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; SMALL64-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; SMALL64-LABEL:  L..C3:
-; SMALL64-NEXT:  .tc TIUninit[TC],TIUninit[UL]@ld
+; SMALL64-NEXT:  .tc _Renamed..5f24__TLSLD.TIUninit[TC],TIUninit[UL]@ld
+; SMALL64-NEXT:  .rename _Renamed..5f24__TLSLD.TIUninit[TC],"TIUninit"
 ; SMALL64-LABEL:  L..C4:
-; SMALL64-NEXT:  .tc TIInit[TC],TIInit[TL]@ld
+; SMALL64-NEXT:  .tc _Renamed..5f24__TLSLD.TIInit[TC],TIInit[TL]@ld
+; SMALL64-NEXT:  .rename _Renamed..5f24__TLSLD.TIInit[TC],"TIInit"
 ; SMALL64-LABEL:  L..C5:
 ; SMALL64-NEXT:  .tc .TWInit[TC],TWInit[TL]@m
 ; SMALL64-LABEL:  L..C6:
@@ -744,9 +750,11 @@ entry:
 ; LARGE64-NEXT:  .tc _Renamed..5f24__TLSML[TC],_Renamed..5f24__TLSML[TC]@ml
 ; LARGE64-NEXT:  .rename _Renamed..5f24__TLSML[TC],"_$TLSML"
 ; LARGE64-LABEL:  L..C3:
-; LARGE64-NEXT:  .tc TIUninit[TE],TIUninit[UL]@ld
+; LARGE64-NEXT:  .tc _Renamed..5f24__TLSLD.TIUninit[TE],TIUninit[UL]@ld
+; LARGE64-NEXT:  .rename _Renamed..5f24__TLSLD.TIUninit[TE],"TIUninit"
 ; LARGE64-LABEL:  L..C4:
-; LARGE64-NEXT:  .tc TIInit[TE],TIInit[TL]@ld
+; LARGE64-NEXT:  .tc _Renamed..5f24__TLSLD.TIInit[TE],TIInit[TL]@ld
+; LARGE64-NEXT:  .rename _Renamed..5f24__TLSLD.TIInit[TE],"TIInit"
 ; LARGE64-LABEL:  L..C5:
 ; LARGE64-NEXT:  .tc .TWInit[TE],TWInit[TL]@m
 ; LARGE64-LABEL:  L..C6:
diff --git a/llvm/test/CodeGen/PowerPC/aix-tls-ld-unqualified-symbols.ll b/llvm/test/CodeGen/PowerPC/aix-tls-ld-unqualified-symbols.ll
new file mode 100644
index 00000000000000..d92375730a95a7
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/aix-tls-ld-unqualified-symbols.ll
@@ -0,0 +1,362 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec -mtriple powerpc64-ibm-aix-xcoff \
+; RUN:     --code-model=small < %s | FileCheck %s --check-prefixes=SMALL64,COMMON
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec -mtriple powerpc64-ibm-aix-xcoff \
+; RUN:     --code-model=large < %s | FileCheck %s --check-prefixes=LARGE64,COMMON
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec -mtriple powerpc-ibm-aix-xcoff \
+; RUN:     --code-model=small < %s | FileCheck %s --check-prefixes=SMALL32,COMMON
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec -mtriple powerpc-ibm-aix-xcoff \
+; RUN:     --code-model=large < %s | FileCheck %s --check-prefixes=LARGE32,COMMON
+
+@_$TLSLD. = thread_local(localdynamic) global i32 42, align 4
+@_$TLSLD.a = thread_local(localdynamic) global i32 42, align 4
+@__$TLSLD. = internal thread_local(localdynamic) global i32 42, align 4
+@_$TLSLD._$TLSLD. = internal thread_local(localdynamic) global i32 42, align 4
+
+declare nonnull ptr @llvm.threadlocal.address.p0(ptr nonnull)
+
+define i32 @loadSym1() {
+; SMALL64-LABEL: loadSym1:
+; SMALL64:       # %bb.0: # %entry
+; SMALL64-NEXT:    mflr 0
+; SMALL64-NEXT:    stdu 1, -48(1)
+; SMALL64-NEXT:    ld 3, L..C0(2) # target-flags(ppc-tlsldm) @"_$TLSML"
+; SMALL64-NEXT:    std 0, 64(1)
+; SMALL64-NEXT:    bla .__tls_get_mod[PR]
+; SMALL64-NEXT:    ld 4, L..C1(2) # target-flags(ppc-tlsld) @"_$TLSLD."
+; SMALL64-NEXT:    lwzx 3, 3, 4
+; SMALL64-NEXT:    addi 1, 1, 48
+; SMALL64-NEXT:    ld 0, 16(1)
+; SMALL64-NEXT:    mtlr 0
+; SMALL64-NEXT:    blr
+;
+; LARGE64-LABEL: loadSym1:
+; LARGE64:       # %bb.0: # %entry
+; LARGE64-NEXT:    mflr 0
+; LARGE64-NEXT:    stdu 1, -48(1)
+; LARGE64-NEXT:    addis 3, L..C0@u(2)
+; LARGE64-NEXT:    addis 6, L..C1@u(2)
+; LARGE64-NEXT:    std 0, 64(1)
+; LARGE64-NEXT:    ld 3, L..C0@l(3)
+; LARGE64-NEXT:    bla .__tls_get_mod[PR]
+; LARGE64-NEXT:    ld 4, L..C1@l(6)
+; LARGE64-NEXT:    lwzx 3, 3, 4
+; LARGE64-NEXT:    addi 1, 1, 48
+; LARGE64-NEXT:    ld 0, 16(1)
+; LARGE64-NEXT:    mtlr 0
+; LARGE64-NEXT:    blr
+;
+; SMALL32-LABEL: loadSym1:
+; SMALL32:       # %bb.0: # %entry
+; SMALL32-NEXT:    mflr 0
+; SMALL32-NEXT:    stwu 1, -32(1)
+; SMALL32-NEXT:    lwz 3, L..C0(2) # target-flags(ppc-tlsldm) @"_$TLSML"
+; SMALL32-NEXT:    stw 0, 40(1)
+; SMALL32-NEXT:    bla .__tls_get_mod[PR]
+; SMALL32-NEXT:    lwz 4, L..C1(2) # target-flags(ppc-tlsld) @"_$TLSLD."
+; SMALL32-NEXT:    lwzx 3, 3, 4
+; SMALL32-NEXT:    addi 1, 1, 32
+; SMALL32-NEXT:    lwz 0, 8(1)
+; SMALL32-NEXT:    mtlr 0
+; SMALL32-NEXT:    blr
+;
+; LARGE32-LABEL: loadSym1:
+; LARGE32:       # %bb.0: # %entry
+; LARGE32-NEXT:    mflr 0
+; LARGE32-NEXT:    stwu 1, -32(1)
+; LARGE32-NEXT:    stw 0, 40(1)
+; LARGE32-NEXT:    addis 6, L..C0@u(2)
+; LARGE32-NEXT:    addis 3, L..C1@u(2)
+; LARGE32-NEXT:    lwz 3, L..C1@l(3)
+; LARGE32-NEXT:    bla .__tls_get_mod[PR]
+; LARGE32-NEXT:    lwz 4, L..C0@l(6)
+; LARGE32-NEXT:    lwzx 3, 3, 4
+; LARGE32-NEXT:    addi 1, 1, 32
+; LARGE32-NEXT:    lwz 0, 8(1)
+; LARGE32-NEXT:    mtlr 0
+; LARGE32-NEXT:    blr
+entry:
+  %0 = tail call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @_$TLSLD.)
+  %1 = load i32, ptr %0, align 4
+  ret i32 %1
+}
+
+define i32 @loadSym2() {
+; SMALL64-LABEL: loadSym2:
+; SMALL64:       # %bb.0: # %entry
+; SMALL64-NEXT:    mflr 0
+; SMALL64-NEXT:    stdu 1, -48(1)
+; SMALL64-NEXT:    ld 3, L..C0(2) # target-flags(ppc-tlsldm) @"_$TLSML"
+; SMALL64-NEXT:    s...
[truncated]

@orcguru
Copy link
Contributor Author

orcguru commented Mar 11, 2024

I will push a change to limit the rename to 64bit only.

@bzEq
Copy link
Collaborator

bzEq commented Mar 12, 2024

so that in following patches we can switch between different TLS models (local-dynamic or initial-exec) for the same TLS GV in different functions within the same module.

Could you please outline what following patches will do?

@orcguru
Copy link
Contributor Author

orcguru commented Mar 12, 2024

Could you please outline what following patches will do?

Under some circumstance (library not loaded by dlopen), TLS initial-exec and local-dynamic access model can be interchanged. Some particular TLS variable could be accessed by different functions in the same object file. We could use some simple heuristic to decide use which TLS model when the variable is accessed in which function.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 14, 2024
@orcguru orcguru changed the title [PowerPC] Rename symbols references by tls-local-dynamic model on AIX [PowerPC] Tune AIX shared library TLS model at function level by heuristic Mar 15, 2024
@@ -80,6 +80,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
bool IsISA3_0 = false;
bool IsISA3_1 = false;
bool HasQuadwordAtomics = false;
bool HasAIXShLibTLSModelHeuristic = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks redundant. Frontend doesn't read this flag to change behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kai, Thank you for pointing out this.

I checked the code and it seems this flag is being used by backend in various places (for example: PPCTargetLowering::LowerGlobalTLSAddressAIX). You referred to another example that this flag is redundant, may I know the details and I will cross check with that scenario?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable defined here is only visible to clang and clang doesn't use this flag further. What backend sees is the feature string and set corresponding variables in backend accordingly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code in PPCTargetLowering::LowerGlobalTLSAddressAIX like

Subtarget.hasAIXSmallLocalExecTLS()

which is generated in PPCGenSubtargetInfo.inc, whose value is initialized by feature string aix-small-local-exec-tls in LLVM IR rather than the variable defined in clang.

@@ -329,6 +329,12 @@ def FeatureAIXLocalExecTLS :
"Produce a TOC-free local-exec TLS sequence for this function "
"for 64-bit AIX">;

def FeatureAIXSharedLibraryTLSModelHeuristic :
SubtargetFeature<"aix-shared-library-tls-model-heuristic",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an optimization, maybe rename to aix-shared-library-tls-model-opt, we perform optimization based on some heuristics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the name to aix-shared-lib-tls-model-opt. Hope that is fine.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
if (Subtarget.hasAIXShLibTLSModelHeuristic() &&
!FuncInfo->isAIXFuncUseInitDone()) {
std::set<const GlobalValue *> TLSGV;
for (SDNode &Node : DAG.allnodes()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One DAG is mapped to a single basic block, I notice your description is about whole function. So the size of TLSGV is smaller than what you really want to count.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emm, I will update test case to expose this issue. I will propose scan over all instructions in my next update. Thank you!

clang/include/clang/Driver/Options.td Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

@orcguru orcguru force-pushed the aix_rename_tls_ld branch from 728851c to 8cd54b7 Compare April 9, 2024 03:15
Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing my comment!
I think I do not have any further comments, so unless if anyone has any other comments LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp Outdated Show resolved Hide resolved
@orcguru
Copy link
Contributor Author

orcguru commented Apr 12, 2024

Since #86641 has been merged, I will continue work on this patch to handle the interactions of these two flags.

Mean while I'm waiting for more review comments from experts. Thank you!

Comment on lines 5024 to 5035
HelpText<"For shared library loaded with the main program, use heuristics to "
"tune the TLS model at the function level (AIX 64-bit only).">;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not describe the scope of what TLS accesses are modified (but it should). I have not looked through the rest of the code yet, but the answer should be local-dynamic accesses may be changed to initial-exec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did update the description. Please let me know if another statement is better. Thank you!

Copy link
Collaborator

@hubert-reinterpretcast hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments for rationale re: scoping to LD -> IE only (and not performing IE -> LD).

if (const GlobalValue *GV =
dyn_cast<GlobalValue>(II->getOperand(0))) {
TLSModel::Model GVModel = TM.getTLSModel(GV);
if (GVModel == TLSModel::InitialExec ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only count the number of local-dynamic accesses.

Rationale:
The option can be taken as providing the information that the shared object is loaded with the executable (not dlopened later). In that case, all variables accessed using the local-dynamic model can be accessed using initial-exec; however, there would still be variables accessed using initial-exec that cannot be accessed using local-dynamic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the guidance. I have limited the transformation.

Comment on lines 3401 to 3404
if (TLSGVCnt == 1)
FuncInfo->setAIXFuncUseTLSIE();
else if (TLSGVCnt > 1)
FuncInfo->setAIXFuncUseTLSLD();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should need to set only one thing (to represent "use IE instead of LD"). The threshold number should not be repeated in more than one place.

Additionally, the threshold number may need to be increased (tests via experiments).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added target hidden flag PPCAIXTLSModelOptUseIEForLDLimit in PPCISelLowering.cpp, and its initial value will be 1. Also updated test case to show the effect of change this parameter.

@orcguru orcguru force-pushed the aix_rename_tls_ld branch 2 times, most recently from 8925ef8 to 7fdf4f7 Compare April 23, 2024 02:37
@orcguru orcguru changed the title [PowerPC] Tune AIX shared library TLS model at function level by heuristic [PowerPC] Tune AIX shared library TLS model at function level Apr 23, 2024
Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after all the updates. Thanks!

DAG.getMachineFunction().getInfo<PPCFunctionInfo>();
if (!FuncInfo->isAIXFuncTLSModelOptInitDone()) {
SmallPtrSet<const GlobalValue *, 8> TLSGV;
// Iterate over all instructions within current function, collect all TLS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit on comment:

Suggested change
// Iterate over all instructions within current function, collect all TLS
// Iterate over all instructions within the current function and collect all TLS

orcguru added 10 commits May 8, 2024 20:25
On AIX, rename those tls-local-dynamic referenced TOC symbols, so that in
following patches we can switch between different TLS models (local-dynamic
or initial-exec) for the same TLS GV in different functions within the same
module.
(1) Address issue that only the first BB get checked inside
`LowerGlobalTLSAddressAIX`, also added test case to expose the issue.
(2) Change feature name from `aix-shared-library-tls-model-heuristic`
to `aix-shared-lib-tls-model-opt`.
(3) Update naming convention in test cases to avoid numbered
identifiers.
(4) Update comments and code simplifications.
`Subtarget.hasAIXShLibTLSModelOpt()`.
(1) Update Options.td to make it clear this is about the transform from LD to IE.
(2) Limit transfrom from LD to IE only.
(3) Add target hidden flag to control Limit count of the transformation.
(4) Update test case.
aix-small-local-dynamic-tls interactions.
(2) Since TM.getTLSModel() result may need further check once this
is enabled, update GetSymbolRef() logic.
(3) Removed "heuristic" from comments.
@orcguru orcguru force-pushed the aix_rename_tls_ld branch from d0553e8 to 305d36f Compare May 9, 2024 00:27
@orcguru orcguru merged commit ea126ae into llvm:main May 9, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants