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.deshpa...@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.deshpa...@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.deshpa...@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.deshpa...@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.deshpa...@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() << "\"" << " && "; } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits