[clang] [Clang] Improve EmitClangAttrSpellingListIndex (PR #114899)
Chinmay Deshpande via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 7 11:24:48 PST 2024
https://github.com/chinmaydd updated https://github.com/llvm/llvm-project/pull/114899
>From cf4d70c23b2896483b452622300aa4c8c41c01e5 Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <ChinmayDiwakar.Deshpande at amd.com>
Date: Fri, 1 Nov 2024 19:49:52 -0400
Subject: [PATCH 1/5] [Clang] Improve EmitClangAttrSpellingListIndex
EmitClangAttrSpellingListIndex() performs a lot of
unnecessary string comparisons which is wasteful in time and space. This
commit attempts to refactor this method to be more performant.
---
.../include/clang/Basic/AttributeCommonInfo.h | 10 ++
clang/lib/Basic/Attributes.cpp | 28 +++++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 95 ++++++++++++++++++-
3 files changed, 126 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index 5f024b4b5fd782..834b3ca62adced 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -67,6 +67,16 @@ class AttributeCommonInfo {
IgnoredAttribute,
UnknownAttribute,
};
+ enum Scope {
+ SC_NONE,
+ SC_CLANG,
+ SC_GNU,
+ SC_MSVC,
+ SC_OMP,
+ SC_HLSL,
+ SC_GSL,
+ SC_RISCV
+ };
private:
const IdentifierInfo *AttrName = nullptr;
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 867d241a2cf847..6f5a70674cbd4b 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -16,6 +16,7 @@
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/ParsedAttrInfo.h"
#include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/StringMap.h"
using namespace clang;
@@ -153,12 +154,35 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
normalizeName(getAttrName(), getScopeName(), getSyntax()));
}
+static const llvm::StringMap<AttributeCommonInfo::Scope> ScopeMap = {
+ {"", AttributeCommonInfo::SC_NONE},
+ {"clang", AttributeCommonInfo::SC_CLANG},
+ {"gnu", AttributeCommonInfo::SC_GNU},
+ {"msvc", AttributeCommonInfo::SC_MSVC},
+ {"omp", AttributeCommonInfo::SC_OMP},
+ {"hlsl", AttributeCommonInfo::SC_HLSL},
+ {"gsl", AttributeCommonInfo::SC_GSL},
+ {"riscv", AttributeCommonInfo::SC_RISCV}};
+
+AttributeCommonInfo::Scope
+getScopeFromNormalizedScopeName(const StringRef ScopeName) {
+ auto It = ScopeMap.find(ScopeName);
+ if (It == ScopeMap.end()) {
+ llvm_unreachable("Unknown normalized scope name. Shouldn't get here");
+ }
+
+ return It->second;
+}
+
unsigned AttributeCommonInfo::calculateAttributeSpellingListIndex() const {
// Both variables will be used in tablegen generated
// attribute spell list index matching code.
auto Syntax = static_cast<AttributeCommonInfo::Syntax>(getSyntax());
- StringRef Scope = normalizeAttrScopeName(getScopeName(), Syntax);
- StringRef Name = normalizeAttrName(getAttrName(), Scope, Syntax);
+ StringRef ScopeName = normalizeAttrScopeName(getScopeName(), Syntax);
+ StringRef Name = normalizeAttrName(getAttrName(), ScopeName, Syntax);
+
+ AttributeCommonInfo::Scope ComputedScope =
+ getScopeFromNormalizedScopeName(ScopeName);
#include "clang/Sema/AttrSpellingListIndex.inc"
}
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 5a80c8c0b7ad36..4db75a92639ad6 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -20,6 +20,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/StringSwitch.h"
@@ -3843,11 +3844,95 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
const Record &R = *I.second;
std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(R);
OS << " case AT_" << I.first << ": {\n";
- for (unsigned I = 0; I < Spellings.size(); ++ I) {
- OS << " if (Name == \"" << Spellings[I].name() << "\" && "
- << "getSyntax() == AttributeCommonInfo::AS_" << Spellings[I].variety()
- << " && Scope == \"" << Spellings[I].nameSpace() << "\")\n"
- << " return " << I << ";\n";
+
+ // If there are none or one spelling to check, resort to the default
+ // behavior of returning index as 0.
+ if (Spellings.size() <= 1) {
+ OS << " return 0;\n";
+ OS << " break;\n";
+ OS << " }\n";
+ continue;
+ }
+
+ bool HasSingleUniqueSpellingName = true;
+ StringMap<std::vector<const FlattenedSpelling *>> SpellingMap;
+
+ StringRef FirstName = Spellings.front().name();
+ for (const auto &S : Spellings) {
+ StringRef Name = S.name();
+ if (Name != FirstName)
+ HasSingleUniqueSpellingName = false;
+ SpellingMap[Name].push_back(&S);
+ }
+
+ // If parsed attribute has only one possible spelling name, only compare
+ // syntax and scope.
+ if (HasSingleUniqueSpellingName) {
+ for (const auto &[Idx, S] : enumerate(SpellingMap[FirstName])) {
+ OS << " if (getSyntax() == AttributeCommonInfo::AS_" << S->variety();
+
+ std::string ScopeStr = "AttributeCommonInfo::SC_";
+ if (S->nameSpace() == "")
+ ScopeStr += "NONE";
+ else
+ ScopeStr += S->nameSpace().upper();
+
+ OS << " && ComputedScope == " << ScopeStr << ")\n"
+ << " return " << Idx << ";\n";
+ }
+ } else {
+ size_t Idx = 0;
+ for (const auto &MapEntry : SpellingMap) {
+ StringRef Name = MapEntry.first();
+ const std::vector<const FlattenedSpelling *> &Cases = SpellingMap[Name];
+
+ if (Cases.size() > 1) {
+ // For names with multiple possible cases, emit an enclosing if such
+ // that the name is compared against only once. Eg:
+ //
+ // if (Name == "always_inline") {
+ // if (getSyntax() == AttributeCommonInfo::AS_GNU &&
+ // ComputedScope == AttributeCommonInfo::SC_None)
+ // return 0;
+ // ...
+ // }
+ OS << " if (Name == \"" << Name << "\") {\n";
+ for (const auto &S : SpellingMap[Name]) {
+ OS << " if (getSyntax() == AttributeCommonInfo::AS_"
+ << S->variety();
+ std::string ScopeStr = "AttributeCommonInfo::SC_";
+ if (S->nameSpace() == "")
+ ScopeStr += "NONE";
+ else
+ ScopeStr += S->nameSpace().upper();
+
+ OS << " && ComputedScope == " << ScopeStr << ")\n"
+ << " return " << Idx << ";\n";
+ Idx++;
+ }
+ OS << " }\n";
+ } else {
+ // If there is only possible case for the spelling name, no need of
+ // enclosing if. Eg.
+ //
+ // if (Name == "__forceinline" &&
+ // getSyntax() == AttributeCommonInfo::AS_Keyword
+ // && ComputedScope == AttributeCommonInfo::SC_NONE)
+ // return 5;
+ const FlattenedSpelling *S = Cases.front();
+ OS << " if (Name == \"" << Name << "\"";
+ OS << " && getSyntax() == AttributeCommonInfo::AS_" << S->variety();
+ std::string ScopeStr = "AttributeCommonInfo::SC_";
+ if (S->nameSpace() == "")
+ ScopeStr += "NONE";
+ else
+ ScopeStr += S->nameSpace().upper();
+
+ OS << " && ComputedScope == " << ScopeStr << ")\n"
+ << " return " << Idx << ";\n";
+ Idx++;
+ }
+ }
}
OS << " break;\n";
>From 29700415c58a03414685fe3c8b809f08932e481a Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <ChinmayDiwakar.Deshpande at amd.com>
Date: Mon, 4 Nov 2024 22:23:11 -0500
Subject: [PATCH 2/5] [Clang] Address comments and fix bugs
---
clang/lib/Basic/Attributes.cpp | 3 +-
clang/utils/TableGen/ClangAttrEmitter.cpp | 61 +++++++++++++----------
2 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 6f5a70674cbd4b..924e138c5a2650 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -16,7 +16,6 @@
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/ParsedAttrInfo.h"
#include "clang/Basic/TargetInfo.h"
-#include "llvm/ADT/StringMap.h"
using namespace clang;
@@ -154,7 +153,7 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
normalizeName(getAttrName(), getScopeName(), getSyntax()));
}
-static const llvm::StringMap<AttributeCommonInfo::Scope> ScopeMap = {
+const std::map<StringRef, AttributeCommonInfo::Scope> ScopeMap = {
{"", AttributeCommonInfo::SC_NONE},
{"clang", AttributeCommonInfo::SC_CLANG},
{"gnu", AttributeCommonInfo::SC_GNU},
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 4db75a92639ad6..ce91ced1676ca2 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3848,9 +3848,9 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
// If there are none or one spelling to check, resort to the default
// behavior of returning index as 0.
if (Spellings.size() <= 1) {
- OS << " return 0;\n";
- OS << " break;\n";
- OS << " }\n";
+ OS << " return 0;\n"
+ << " break;\n"
+ << " }\n";
continue;
}
@@ -3869,22 +3869,26 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
// syntax and scope.
if (HasSingleUniqueSpellingName) {
for (const auto &[Idx, S] : enumerate(SpellingMap[FirstName])) {
- OS << " if (getSyntax() == AttributeCommonInfo::AS_" << S->variety();
+ OS << " if (getSyntax() == AttributeCommonInfo::AS_" << S->variety()
+ << " && ComputedScope == ";
- std::string ScopeStr = "AttributeCommonInfo::SC_";
if (S->nameSpace() == "")
- ScopeStr += "NONE";
+ OS << "AttributeCommonInfo::SC_NONE";
else
- ScopeStr += S->nameSpace().upper();
+ OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
- OS << " && ComputedScope == " << ScopeStr << ")\n"
+ OS << ")\n"
<< " return " << Idx << ";\n";
}
} else {
size_t Idx = 0;
- for (const auto &MapEntry : SpellingMap) {
- StringRef Name = MapEntry.first();
- const std::vector<const FlattenedSpelling *> &Cases = SpellingMap[Name];
+ StringMap<bool> Completed;
+ for (const auto &Spell : Spellings) {
+ if (Completed.contains(Spell.name()))
+ continue;
+
+ const std::vector<const FlattenedSpelling *> &Cases =
+ SpellingMap[Spell.name()];
if (Cases.size() > 1) {
// For names with multiple possible cases, emit an enclosing if such
@@ -3896,17 +3900,17 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
// return 0;
// ...
// }
- OS << " if (Name == \"" << Name << "\") {\n";
- for (const auto &S : SpellingMap[Name]) {
+ OS << " if (Name == \"" << Spell.name() << "\") {\n";
+ for (const auto &S : SpellingMap[Spell.name()]) {
OS << " if (getSyntax() == AttributeCommonInfo::AS_"
- << S->variety();
- std::string ScopeStr = "AttributeCommonInfo::SC_";
+ << S->variety() << " && ComputedScope == ";
+
if (S->nameSpace() == "")
- ScopeStr += "NONE";
+ OS << "AttributeCommonInfo::SC_NONE";
else
- ScopeStr += S->nameSpace().upper();
+ OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
- OS << " && ComputedScope == " << ScopeStr << ")\n"
+ OS << ")\n"
<< " return " << Idx << ";\n";
Idx++;
}
@@ -3920,27 +3924,30 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
// && ComputedScope == AttributeCommonInfo::SC_NONE)
// return 5;
const FlattenedSpelling *S = Cases.front();
- OS << " if (Name == \"" << Name << "\"";
- OS << " && getSyntax() == AttributeCommonInfo::AS_" << S->variety();
+ OS << " if (Name == \"" << Spell.name() << "\""
+ << " && getSyntax() == AttributeCommonInfo::AS_" << S->variety()
+ << " && ComputedScope == ";
+
std::string ScopeStr = "AttributeCommonInfo::SC_";
if (S->nameSpace() == "")
- ScopeStr += "NONE";
+ OS << "AttributeCommonInfo::SC_NONE";
else
- ScopeStr += S->nameSpace().upper();
+ OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
- OS << " && ComputedScope == " << ScopeStr << ")\n"
+ OS << ")\n"
<< " return " << Idx << ";\n";
Idx++;
}
+ Completed[Spell.name()] = true;
}
}
- OS << " break;\n";
- OS << " }\n";
+ OS << " break;\n"
+ << " }\n";
}
- OS << " }\n";
- OS << " return 0;\n";
+ OS << " }\n"
+ << " return 0;\n";
}
// Emits code used by RecursiveASTVisitor to visit attributes
>From 95b75698087a242918830f738d9e08c04fc89250 Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <ChinmayDiwakar.Deshpande at amd.com>
Date: Tue, 5 Nov 2024 17:13:20 -0500
Subject: [PATCH 3/5] [Clang] Change up algorithm
---
.../include/clang/Basic/AttributeCommonInfo.h | 11 +-
clang/lib/Basic/Attributes.cpp | 22 ++--
clang/utils/TableGen/ClangAttrEmitter.cpp | 106 ++++++------------
3 files changed, 48 insertions(+), 91 deletions(-)
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index 834b3ca62adced..11c64547721739 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -67,16 +67,7 @@ class AttributeCommonInfo {
IgnoredAttribute,
UnknownAttribute,
};
- enum Scope {
- SC_NONE,
- SC_CLANG,
- SC_GNU,
- SC_MSVC,
- SC_OMP,
- SC_HLSL,
- SC_GSL,
- SC_RISCV
- };
+ enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV };
private:
const IdentifierInfo *AttrName = nullptr;
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 924e138c5a2650..5bf287ce2af4b4 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -17,6 +17,8 @@
#include "clang/Basic/ParsedAttrInfo.h"
#include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/StringMap.h"
+
using namespace clang;
static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
@@ -153,18 +155,18 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
normalizeName(getAttrName(), getScopeName(), getSyntax()));
}
-const std::map<StringRef, AttributeCommonInfo::Scope> ScopeMap = {
- {"", AttributeCommonInfo::SC_NONE},
- {"clang", AttributeCommonInfo::SC_CLANG},
- {"gnu", AttributeCommonInfo::SC_GNU},
- {"msvc", AttributeCommonInfo::SC_MSVC},
- {"omp", AttributeCommonInfo::SC_OMP},
- {"hlsl", AttributeCommonInfo::SC_HLSL},
- {"gsl", AttributeCommonInfo::SC_GSL},
- {"riscv", AttributeCommonInfo::SC_RISCV}};
+const llvm::StringMap<AttributeCommonInfo::Scope> ScopeMap = {
+ {"", AttributeCommonInfo::Scope::NONE},
+ {"clang", AttributeCommonInfo::Scope::CLANG},
+ {"gnu", AttributeCommonInfo::Scope::GNU},
+ {"msvc", AttributeCommonInfo::Scope::MSVC},
+ {"omp", AttributeCommonInfo::Scope::OMP},
+ {"hlsl", AttributeCommonInfo::Scope::HLSL},
+ {"gsl", AttributeCommonInfo::Scope::GSL},
+ {"riscv", AttributeCommonInfo::Scope::RISCV}};
AttributeCommonInfo::Scope
-getScopeFromNormalizedScopeName(const StringRef ScopeName) {
+getScopeFromNormalizedScopeName(StringRef ScopeName) {
auto It = ScopeMap.find(ScopeName);
if (It == ScopeMap.end()) {
llvm_unreachable("Unknown normalized scope name. Shouldn't get here");
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index ce91ced1676ca2..cde090d59b9dac 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3854,91 +3854,55 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
continue;
}
- bool HasSingleUniqueSpellingName = true;
- StringMap<std::vector<const FlattenedSpelling *>> SpellingMap;
-
- StringRef FirstName = Spellings.front().name();
- for (const auto &S : Spellings) {
- StringRef Name = S.name();
- if (Name != FirstName)
- HasSingleUniqueSpellingName = false;
- SpellingMap[Name].push_back(&S);
- }
-
- // If parsed attribute has only one possible spelling name, only compare
- // syntax and scope.
- if (HasSingleUniqueSpellingName) {
- for (const auto &[Idx, S] : enumerate(SpellingMap[FirstName])) {
- OS << " if (getSyntax() == AttributeCommonInfo::AS_" << S->variety()
+ std::vector<StringRef> Names;
+ llvm::transform(Spellings, std::back_inserter(Names),
+ [](const FlattenedSpelling &FS) { return FS.name(); });
+ llvm::sort(Names);
+ Names.erase(llvm::unique(Names), Names.end());
+
+ for (const auto &[Idx, FS] : enumerate(Spellings)) {
+ if (Names.size() == 1) {
+ OS << " if (getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
<< " && ComputedScope == ";
- if (S->nameSpace() == "")
- OS << "AttributeCommonInfo::SC_NONE";
+ if (FS.nameSpace() == "")
+ OS << "AttributeCommonInfo::Scope::NONE";
else
- OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
+ OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
OS << ")\n"
<< " return " << Idx << ";\n";
- }
- } else {
- size_t Idx = 0;
- StringMap<bool> Completed;
- for (const auto &Spell : Spellings) {
- if (Completed.contains(Spell.name()))
- continue;
+ } else {
+ SmallVector<StringRef, 6> SameLenNames;
+ llvm::copy_if(
+ Names, std::back_inserter(SameLenNames),
+ [&](StringRef N) { return N.size() == FS.name().size(); });
+
+ if (SameLenNames.size() == 1) {
+ OS << " if (Name.size() == " << FS.name().size()
+ << " && getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
+ << " && ComputedScope == ";
- const std::vector<const FlattenedSpelling *> &Cases =
- SpellingMap[Spell.name()];
-
- if (Cases.size() > 1) {
- // For names with multiple possible cases, emit an enclosing if such
- // that the name is compared against only once. Eg:
- //
- // if (Name == "always_inline") {
- // if (getSyntax() == AttributeCommonInfo::AS_GNU &&
- // ComputedScope == AttributeCommonInfo::SC_None)
- // return 0;
- // ...
- // }
- OS << " if (Name == \"" << Spell.name() << "\") {\n";
- for (const auto &S : SpellingMap[Spell.name()]) {
- OS << " if (getSyntax() == AttributeCommonInfo::AS_"
- << S->variety() << " && ComputedScope == ";
-
- if (S->nameSpace() == "")
- OS << "AttributeCommonInfo::SC_NONE";
- else
- OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
-
- OS << ")\n"
- << " return " << Idx << ";\n";
- Idx++;
- }
- OS << " }\n";
+ if (FS.nameSpace() == "")
+ OS << "AttributeCommonInfo::Scope::NONE";
+ else
+ OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
+
+ OS << ")\n"
+ << " return " << Idx << ";\n";
} else {
- // If there is only possible case for the spelling name, no need of
- // enclosing if. Eg.
- //
- // if (Name == "__forceinline" &&
- // getSyntax() == AttributeCommonInfo::AS_Keyword
- // && ComputedScope == AttributeCommonInfo::SC_NONE)
- // return 5;
- const FlattenedSpelling *S = Cases.front();
- OS << " if (Name == \"" << Spell.name() << "\""
- << " && getSyntax() == AttributeCommonInfo::AS_" << S->variety()
+ OS << " if (Name == \"" << FS.name() << "\""
+ << " && getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
<< " && ComputedScope == ";
- std::string ScopeStr = "AttributeCommonInfo::SC_";
- if (S->nameSpace() == "")
- OS << "AttributeCommonInfo::SC_NONE";
+ if (FS.nameSpace() == "")
+ OS << "AttributeCommonInfo::Scope::NONE";
else
- OS << "AttributeCommonInfo::SC_" + S->nameSpace().upper();
+ OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
OS << ")\n"
- << " return " << Idx << ";\n";
- Idx++;
+ << " return " << Idx << ";\n";
}
- Completed[Spell.name()] = true;
}
}
>From 87ac151d2f3fb71220f1262e330245e56bb49287 Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <ChinmayDiwakar.Deshpande at amd.com>
Date: Wed, 6 Nov 2024 12:37:17 -0500
Subject: [PATCH 4/5] [Clang] Add FIXME note and address comments
---
clang/lib/Basic/Attributes.cpp | 4 +-
clang/utils/TableGen/ClangAttrEmitter.cpp | 48 ++++++++---------------
2 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 5bf287ce2af4b4..d569b321a2bc2d 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -168,9 +168,7 @@ const llvm::StringMap<AttributeCommonInfo::Scope> ScopeMap = {
AttributeCommonInfo::Scope
getScopeFromNormalizedScopeName(StringRef ScopeName) {
auto It = ScopeMap.find(ScopeName);
- if (It == ScopeMap.end()) {
- llvm_unreachable("Unknown normalized scope name. Shouldn't get here");
- }
+ assert(It != ScopeMap.end());
return It->second;
}
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index cde090d59b9dac..53cbb2b634e1d5 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3862,16 +3862,7 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
for (const auto &[Idx, FS] : enumerate(Spellings)) {
if (Names.size() == 1) {
- OS << " if (getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
- << " && ComputedScope == ";
-
- if (FS.nameSpace() == "")
- OS << "AttributeCommonInfo::Scope::NONE";
- else
- OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
-
- OS << ")\n"
- << " return " << Idx << ";\n";
+ OS << " if (";
} else {
SmallVector<StringRef, 6> SameLenNames;
llvm::copy_if(
@@ -3879,31 +3870,26 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
[&](StringRef N) { return N.size() == FS.name().size(); });
if (SameLenNames.size() == 1) {
- OS << " if (Name.size() == " << FS.name().size()
- << " && getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
- << " && ComputedScope == ";
-
- if (FS.nameSpace() == "")
- OS << "AttributeCommonInfo::Scope::NONE";
- else
- OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
-
- OS << ")\n"
- << " return " << Idx << ";\n";
+ OS << " if (Name.size() == " << FS.name().size() << " && ";
} else {
+ // FIXME: We currently fall back to comparing entire strings if there
+ // are 2 or more spelling names with the same length. This can be
+ // optimized to check only for the the first differing character
+ // between them instead.
OS << " if (Name == \"" << FS.name() << "\""
- << " && getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
- << " && ComputedScope == ";
-
- if (FS.nameSpace() == "")
- OS << "AttributeCommonInfo::Scope::NONE";
- else
- OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
-
- OS << ")\n"
- << " return " << Idx << ";\n";
+ << " && ";
}
}
+
+ OS << "getSyntax() == AttributeCommonInfo::AS_" << FS.variety()
+ << " && ComputedScope == ";
+ if (FS.nameSpace() == "")
+ OS << "AttributeCommonInfo::Scope::NONE";
+ else
+ OS << "AttributeCommonInfo::Scope::" + FS.nameSpace().upper();
+
+ OS << ")\n"
+ << " return " << Idx << ";\n";
}
OS << " break;\n"
>From 6f7c76d0ea1a3f6e327002cfbf3bb81ecf218a28 Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <ChinmayDiwakar.Deshpande at amd.com>
Date: Thu, 7 Nov 2024 14:24:28 -0500
Subject: [PATCH 5/5] [Clang] Address comments
---
clang/lib/Basic/Attributes.cpp | 26 +++++++++++++----------
clang/utils/TableGen/ClangAttrEmitter.cpp | 9 ++++----
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index d569b321a2bc2d..85af7b6bc629e6 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -155,20 +155,24 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
normalizeName(getAttrName(), getScopeName(), getSyntax()));
}
-const llvm::StringMap<AttributeCommonInfo::Scope> ScopeMap = {
- {"", AttributeCommonInfo::Scope::NONE},
- {"clang", AttributeCommonInfo::Scope::CLANG},
- {"gnu", AttributeCommonInfo::Scope::GNU},
- {"msvc", AttributeCommonInfo::Scope::MSVC},
- {"omp", AttributeCommonInfo::Scope::OMP},
- {"hlsl", AttributeCommonInfo::Scope::HLSL},
- {"gsl", AttributeCommonInfo::Scope::GSL},
- {"riscv", AttributeCommonInfo::Scope::RISCV}};
+// Sorted list of attribute scope names
+static constexpr std::pair<StringRef, AttributeCommonInfo::Scope> ScopeList[] =
+ {{"", AttributeCommonInfo::Scope::NONE},
+ {"clang", AttributeCommonInfo::Scope::CLANG},
+ {"gnu", AttributeCommonInfo::Scope::GNU},
+ {"gsl", AttributeCommonInfo::Scope::GSL},
+ {"hlsl", AttributeCommonInfo::Scope::HLSL},
+ {"msvc", AttributeCommonInfo::Scope::MSVC},
+ {"omp", AttributeCommonInfo::Scope::OMP},
+ {"riscv", AttributeCommonInfo::Scope::RISCV}};
AttributeCommonInfo::Scope
getScopeFromNormalizedScopeName(StringRef ScopeName) {
- auto It = ScopeMap.find(ScopeName);
- assert(It != ScopeMap.end());
+ auto It = std::lower_bound(
+ std::begin(ScopeList), std::end(ScopeList), ScopeName,
+ [](const std::pair<StringRef, AttributeCommonInfo::Scope> &Element,
+ StringRef Value) { return Element.first < Value; });
+ assert(It != std::end(ScopeList));
return It->second;
}
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 53cbb2b634e1d5..932cf25f6a7c26 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3861,22 +3861,21 @@ void EmitClangAttrSpellingListIndex(const RecordKeeper &Records,
Names.erase(llvm::unique(Names), Names.end());
for (const auto &[Idx, FS] : enumerate(Spellings)) {
- if (Names.size() == 1) {
- OS << " if (";
- } else {
+ OS << " if (";
+ if (Names.size() > 1) {
SmallVector<StringRef, 6> SameLenNames;
llvm::copy_if(
Names, std::back_inserter(SameLenNames),
[&](StringRef N) { return N.size() == FS.name().size(); });
if (SameLenNames.size() == 1) {
- OS << " if (Name.size() == " << FS.name().size() << " && ";
+ OS << "Name.size() == " << FS.name().size() << " && ";
} else {
// FIXME: We currently fall back to comparing entire strings if there
// are 2 or more spelling names with the same length. This can be
// optimized to check only for the the first differing character
// between them instead.
- OS << " if (Name == \"" << FS.name() << "\""
+ OS << "Name == \"" << FS.name() << "\""
<< " && ";
}
}
More information about the cfe-commits
mailing list