Skip to content

llvm-tli-checker: Remove TLINameList helper struct #142535

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 3, 2025

This avoids subclassing std::vector and a static constructor.
This started as a refactor to make TargetLibraryInfo available during
printing so a custom name could be reported. It turns out this struct
wasn't doing anything, other than providing a hacky way of printing the
standard name instead of the target's custom name. Just remove this and
stop hacking on the TargetLibraryInfo to falsely report the function
is available later.

This avoids subclassing std::vector and a static constructor.
This started as a refactor to make TargetLibraryInfo available during
printing so a custom name could be reported. It turns out this struct
wasn't doing anything, other than providing a hacky way of printing the
standard name instead of the target's custom name. Just remove this and
stop hacking on the TargetLibraryInfo to falsely report the function
is available later.
Copy link
Contributor Author

arsenm commented Jun 3, 2025

@arsenm arsenm added the llvm-tools All llvm tools that do not have corresponding tag label Jun 3, 2025 — with Graphite App
@arsenm arsenm marked this pull request as ready for review June 3, 2025 05:59
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Jun 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 3, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Matt Arsenault (arsenm)

Changes

This avoids subclassing std::vector and a static constructor.
This started as a refactor to make TargetLibraryInfo available during
printing so a custom name could be reported. It turns out this struct
wasn't doing anything, other than providing a hacky way of printing the
standard name instead of the target's custom name. Just remove this and
stop hacking on the TargetLibraryInfo to falsely report the function
is available later.


Full diff: https://github.com/llvm/llvm-project/pull/142535.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/TargetLibraryInfo.h (+6)
  • (modified) llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp (+30-34)
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index 0596ff86b473e..e900d2d4fd80d 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -452,6 +452,12 @@ class TargetLibraryInfo {
     return false;
   }
 
+  /// Return the canonical name for a LibFunc. This should not be used for
+  /// semantic purposes, use getName instead.
+  static StringRef getStandardName(LibFunc F) {
+    return TargetLibraryInfoImpl::StandardNames[F];
+  }
+
   StringRef getName(LibFunc F) const {
     auto State = getState(F);
     if (State == TargetLibraryInfoImpl::Unavailable)
diff --git a/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp b/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
index ca0b424722196..bc20386987cae 100644
--- a/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ b/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -110,43 +110,31 @@ static std::string getPrintableName(StringRef Name) {
   return OutputName;
 }
 
-// Store all the names that TargetLibraryInfo knows about; the bool indicates
-// whether TLI has it marked as "available" for the target of interest.
-// This is a vector to preserve the sorted order for better reporting.
-struct TLINameList : std::vector<std::pair<StringRef, bool>> {
-  // Record all the TLI info in the vector.
-  void initialize(StringRef TargetTriple);
-  // Print out what we found.
-  void dump();
-};
-static TLINameList TLINames;
-
-void TLINameList::initialize(StringRef TargetTriple) {
-  Triple T(TargetTriple);
-  TargetLibraryInfoImpl TLII(T);
-  TargetLibraryInfo TLI(TLII);
+static void reportNumberOfEntries(const TargetLibraryInfo &TLI,
+                                  StringRef TargetTriple) {
+  unsigned NumAvailable = 0;
 
-  reserve(LibFunc::NumLibFuncs);
-  size_t NumAvailable = 0;
+  // Assume this gets called after initialize(), so we have the above line of
+  // output as a header.  So, for example, no need to repeat the triple.
   for (unsigned FI = 0; FI != LibFunc::NumLibFuncs; ++FI) {
-    LibFunc LF = (LibFunc)FI;
-    bool Available = TLI.has(LF);
-    // getName returns names only for available funcs.
-    TLII.setAvailable(LF);
-    emplace_back(TLI.getName(LF), Available);
-    if (Available)
+    if (TLI.has(static_cast<LibFunc>(FI)))
       ++NumAvailable;
   }
+
   outs() << "TLI knows " << LibFunc::NumLibFuncs << " symbols, " << NumAvailable
          << " available for '" << TargetTriple << "'\n";
 }
 
-void TLINameList::dump() {
+static void dumpTLIEntries(const TargetLibraryInfo &TLI) {
   // Assume this gets called after initialize(), so we have the above line of
   // output as a header.  So, for example, no need to repeat the triple.
-  for (auto &TLIName : TLINames) {
-    outs() << (TLIName.second ? "    " : "not ")
-           << "available: " << getPrintableName(TLIName.first) << '\n';
+  for (unsigned FI = 0; FI != LibFunc::NumLibFuncs; ++FI) {
+    LibFunc LF = static_cast<LibFunc>(FI);
+    bool IsAvailable = TLI.has(LF);
+    StringRef FuncName = TargetLibraryInfo::getStandardName(LF);
+
+    outs() << (IsAvailable ? "    " : "not ")
+           << "available: " << getPrintableName(FuncName) << '\n';
   }
 }
 
@@ -271,11 +259,16 @@ int main(int argc, char *argv[]) {
     return 0;
   }
 
-  TLINames.initialize(Args.getLastArgValue(OPT_triple_EQ));
+  StringRef TripleStr = Args.getLastArgValue(OPT_triple_EQ);
+  Triple TargetTriple(TripleStr);
+  TargetLibraryInfoImpl TLII(TargetTriple);
+  TargetLibraryInfo TLI(TLII);
+
+  reportNumberOfEntries(TLI, TripleStr);
 
   // --dump-tli doesn't require any input files.
   if (Args.hasArg(OPT_dump_tli)) {
-    TLINames.dump();
+    dumpTLIEntries(TLI);
     return 0;
   }
 
@@ -321,9 +314,13 @@ int main(int argc, char *argv[]) {
     unsigned TLIdoesntSDKdoes = 0;
     unsigned TLIandSDKboth = 0;
     unsigned TLIandSDKneither = 0;
-    for (auto &TLIName : TLINames) {
-      bool TLIHas = TLIName.second;
-      bool SDKHas = SDKNames.count(TLIName.first) == 1;
+
+    for (unsigned FI = 0; FI != LibFunc::NumLibFuncs; ++FI) {
+      LibFunc LF = static_cast<LibFunc>(FI);
+
+      StringRef TLIName = TLI.getStandardName(LF);
+      bool TLIHas = TLI.has(LF);
+      bool SDKHas = SDKNames.count(TLIName) == 1;
       int Which = int(TLIHas) * 2 + int(SDKHas);
       switch (Which) {
       case 0: ++TLIandSDKneither; break;
@@ -338,8 +335,7 @@ int main(int argc, char *argv[]) {
         constexpr char YesNo[2][4] = {"no ", "yes"};
         constexpr char Indicator[4][3] = {"!!", ">>", "<<", "=="};
         outs() << Indicator[Which] << " TLI " << YesNo[TLIHas] << " SDK "
-               << YesNo[SDKHas] << ": " << getPrintableName(TLIName.first)
-               << '\n';
+               << YesNo[SDKHas] << ": " << getPrintableName(TLIName) << '\n';
       }
     }
 

@pogo59
Copy link
Collaborator

pogo59 commented Jun 4, 2025

I acknowledge dropping the ball on cleaning up this area. I would like to review this (and #142536) but I am out of town this week. I should be able to get to it this weekend.

Copy link
Contributor Author

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

ping

};
static TLINameList TLINames;

void TLINameList::initialize(StringRef TargetTriple) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the overall idea, but I think you would have a better encapsulation with a class that would take the TargetTriple as its constructor, perform initialization and keeping a TLi reference as member, and provide method to dump TLI entries / number of entries. that would avoid leaking some variable in the context. No need of a static variable for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding llvm-tools All llvm tools that do not have corresponding tag
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants