[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG501f92d34382: [llvm] Construct options prefixed name 
at compile-time (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Tooling/Tooling.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -34,30 +34,14 @@
   return OS;
 }
 
-static std::string getOptionSpelling(const Record , size_t ) {
+static std::string getOptionPrefixedName(const Record ) {
   std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
   StringRef Name = R.getValueAsString("Name");
 
-  if (Prefixes.empty()) {
-PrefixLength = 0;
+  if (Prefixes.empty())
 return Name.str();
-  }
-
-  PrefixLength = Prefixes[0].size();
-  return (Twine(Prefixes[0]) + Twine(Name)).str();
-}
 
-static std::string getOptionSpelling(const Record ) {
-  size_t PrefixLength;
-  return getOptionSpelling(R, PrefixLength);
-}
-
-static void emitNameUsingSpelling(raw_ostream , const Record ) {
-  size_t PrefixLength;
-  OS << "llvm::StringLiteral(";
-  write_cstring(
-  OS, StringRef(getOptionSpelling(R, PrefixLength)).substr(PrefixLength));
-  OS << ")";
+  return (Prefixes[0] + Twine(Name)).str();
 }
 
 class MarshallingInfo {
@@ -105,8 +89,6 @@
   }
 
   void emit(raw_ostream ) const {
-write_cstring(OS, StringRef(getOptionSpelling(R)));
-OS << ", ";
 OS << ShouldParse;
 OS << ", ";
 OS << ShouldAlwaysEmit;
@@ -346,8 +328,8 @@
 std::vector RPrefixes = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", ";
 
-// The option string.
-emitNameUsingSpelling(OS, R);
+// The option prefixed name.
+write_cstring(OS, getOptionPrefixedName(R));
 
 // The option identifier name.
 OS << ", " << getOptionName(R);
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 struct OptionWithMarshallingInfo {
-  llvm::StringRef Name;
+  llvm::StringLiteral PrefixedName;
   const char *KeyPath;
   const char *ImpliedCheck;
   const char *ImpliedValue;
@@ -18,20 +18,20 @@
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(   \
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
-HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+PREFIX_TYPE, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
+PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,  \
 DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
 MERGER, EXTRACTOR, TABLE_INDEX)\
-  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
+  {PREFIXED_NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
 
 TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
-  ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
-  ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
-  ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
-  ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
+  ASSERT_EQ(MarshallingTable[0].PrefixedName, "-marshalled-flag-d");
+  ASSERT_EQ(MarshallingTable[1].PrefixedName, "-marshalled-flag-c");
+  ASSERT_EQ(MarshallingTable[2].PrefixedName, "-marshalled-flag-b");
+  ASSERT_EQ(MarshallingTable[3].PrefixedName, "-marshalled-flag-a");
 }
 
 TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -59,7 +59,7 @@
   if ( == )
 return false;
 
-  if (int N = StrCmpOptionName(A.Name, B.Name))
+  if (int N = StrCmpOptionName(A.getName(), B.getName()))
 return N < 0;
 
   for (size_t I = 0, K = std::min(A.Prefixes.size(), B.Prefixes.size()); I != K;
@@ -77,7 +77,7 @@
 
 // Support lower_bound between info and an option name.
 static inline bool operator<(const OptTable::Info , StringRef Name) {
-  return 

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LG! LLVMOption has users in llvm/ clang/ lldb/ lld/ clang-tools-extra/ flang/. 
You'll need to check that all the affected users are migrated...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 548423.
jansvoboda11 added a comment.

Rebase, remove unnecessary changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Tooling/Tooling.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -34,30 +34,14 @@
   return OS;
 }
 
-static std::string getOptionSpelling(const Record , size_t ) {
+static std::string getOptionPrefixedName(const Record ) {
   std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
   StringRef Name = R.getValueAsString("Name");
 
-  if (Prefixes.empty()) {
-PrefixLength = 0;
+  if (Prefixes.empty())
 return Name.str();
-  }
-
-  PrefixLength = Prefixes[0].size();
-  return (Twine(Prefixes[0]) + Twine(Name)).str();
-}
 
-static std::string getOptionSpelling(const Record ) {
-  size_t PrefixLength;
-  return getOptionSpelling(R, PrefixLength);
-}
-
-static void emitNameUsingSpelling(raw_ostream , const Record ) {
-  size_t PrefixLength;
-  OS << "llvm::StringLiteral(";
-  write_cstring(
-  OS, StringRef(getOptionSpelling(R, PrefixLength)).substr(PrefixLength));
-  OS << ")";
+  return (Prefixes[0] + Twine(Name)).str();
 }
 
 class MarshallingInfo {
@@ -105,8 +89,6 @@
   }
 
   void emit(raw_ostream ) const {
-write_cstring(OS, StringRef(getOptionSpelling(R)));
-OS << ", ";
 OS << ShouldParse;
 OS << ", ";
 OS << ShouldAlwaysEmit;
@@ -346,8 +328,8 @@
 std::vector RPrefixes = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", ";
 
-// The option string.
-emitNameUsingSpelling(OS, R);
+// The option prefixed name.
+write_cstring(OS, getOptionPrefixedName(R));
 
 // The option identifier name.
 OS << ", " << getOptionName(R);
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 struct OptionWithMarshallingInfo {
-  llvm::StringRef Name;
+  llvm::StringLiteral PrefixedName;
   const char *KeyPath;
   const char *ImpliedCheck;
   const char *ImpliedValue;
@@ -18,20 +18,20 @@
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(   \
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
-HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+PREFIX_TYPE, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
+PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,  \
 DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
 MERGER, EXTRACTOR, TABLE_INDEX)\
-  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
+  {PREFIXED_NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
 
 TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
-  ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
-  ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
-  ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
-  ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
+  ASSERT_EQ(MarshallingTable[0].PrefixedName, "-marshalled-flag-d");
+  ASSERT_EQ(MarshallingTable[1].PrefixedName, "-marshalled-flag-c");
+  ASSERT_EQ(MarshallingTable[2].PrefixedName, "-marshalled-flag-b");
+  ASSERT_EQ(MarshallingTable[3].PrefixedName, "-marshalled-flag-a");
 }
 
 TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -59,7 +59,7 @@
   if ( == )
 return false;
 
-  if (int N = StrCmpOptionName(A.Name, B.Name))
+  if (int N = StrCmpOptionName(A.getName(), B.getName()))
 return N < 0;
 
   for (size_t I = 0, K = std::min(A.Prefixes.size(), B.Prefixes.size()); I != K;
@@ -77,7 +77,7 @@
 
 // Support lower_bound between info and an option name.
 static inline bool operator<(const OptTable::Info , StringRef Name) {
-  return StrCmpOptionNameIgnoreCase(I.Name, Name) < 0;
+  return StrCmpOptionNameIgnoreCase(I.getName(), Name) < 

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 547339.
jansvoboda11 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Tooling/Tooling.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -34,30 +34,14 @@
   return OS;
 }
 
-static std::string getOptionSpelling(const Record , size_t ) {
+static std::string getOptionPrefixedName(const Record ) {
   std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
   StringRef Name = R.getValueAsString("Name");
 
-  if (Prefixes.empty()) {
-PrefixLength = 0;
+  if (Prefixes.empty())
 return Name.str();
-  }
-
-  PrefixLength = Prefixes[0].size();
-  return (Twine(Prefixes[0]) + Twine(Name)).str();
-}
 
-static std::string getOptionSpelling(const Record ) {
-  size_t PrefixLength;
-  return getOptionSpelling(R, PrefixLength);
-}
-
-static void emitNameUsingSpelling(raw_ostream , const Record ) {
-  size_t PrefixLength;
-  OS << "llvm::StringLiteral(";
-  write_cstring(
-  OS, StringRef(getOptionSpelling(R, PrefixLength)).substr(PrefixLength));
-  OS << ")";
+  return (Prefixes[0] + Twine(Name)).str();
 }
 
 class MarshallingInfo {
@@ -105,8 +89,6 @@
   }
 
   void emit(raw_ostream ) const {
-write_cstring(OS, StringRef(getOptionSpelling(R)));
-OS << ", ";
 OS << ShouldParse;
 OS << ", ";
 OS << ShouldAlwaysEmit;
@@ -303,7 +285,7 @@
 // The option prefix;
 OS << "llvm::ArrayRef()";
 
-// The option string.
+// The option prefixed name.
 OS << ", \"" << R.getValueAsString("Name") << '"';
 
 // The option identifier name.
@@ -346,8 +328,10 @@
 std::vector RPrefixes = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", ";
 
-// The option string.
-emitNameUsingSpelling(OS, R);
+// The option prefixed name.
+OS << "llvm::StringLiteral(";
+write_cstring(OS, getOptionPrefixedName(R));
+OS << ")";
 
 // The option identifier name.
 OS << ", " << getOptionName(R);
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 struct OptionWithMarshallingInfo {
-  llvm::StringRef Name;
+  llvm::StringLiteral PrefixedName;
   const char *KeyPath;
   const char *ImpliedCheck;
   const char *ImpliedValue;
@@ -18,20 +18,20 @@
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(   \
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
-HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+PREFIX_TYPE, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
+PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,  \
 DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
 MERGER, EXTRACTOR, TABLE_INDEX)\
-  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
+  {PREFIXED_NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
 
 TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
-  ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
-  ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
-  ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
-  ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
+  ASSERT_EQ(MarshallingTable[0].PrefixedName, "-marshalled-flag-d");
+  ASSERT_EQ(MarshallingTable[1].PrefixedName, "-marshalled-flag-c");
+  ASSERT_EQ(MarshallingTable[2].PrefixedName, "-marshalled-flag-b");
+  ASSERT_EQ(MarshallingTable[3].PrefixedName, "-marshalled-flag-a");
 }
 
 TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -59,7 +59,7 @@
   if ( == )
 return false;
 
-  if (int N = StrCmpOptionName(A.Name, B.Name))
+  if (int N = StrCmpOptionName(A.getName(), B.getName()))
 return N < 0;
 
   for (size_t I = 0, K = std::min(A.Prefixes.size(), B.Prefixes.size()); I != K;

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D157029#4561519 , @jansvoboda11 
wrote:

> In D157029#4561490 , @MaskRay wrote:
>
>> This increases the size of  `Info` (static data size and static 
>> relocations). In return, some dynamic relocations are saved. Is this a net 
>> win?
>
> If that's a concern, I can remove `Info::Name` and replace its usages with a 
> function call that drops the prefix from prefixed name.

Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 547316.
jansvoboda11 added a comment.

Remove `OptTable::Info::Name`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Tooling/Tooling.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -34,30 +34,14 @@
   return OS;
 }
 
-static std::string getOptionSpelling(const Record , size_t ) {
+static std::string getOptionPrefixedName(const Record ) {
   std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
   StringRef Name = R.getValueAsString("Name");
 
-  if (Prefixes.empty()) {
-PrefixLength = 0;
+  if (Prefixes.empty())
 return Name.str();
-  }
-
-  PrefixLength = Prefixes[0].size();
-  return (Twine(Prefixes[0]) + Twine(Name)).str();
-}
 
-static std::string getOptionSpelling(const Record ) {
-  size_t PrefixLength;
-  return getOptionSpelling(R, PrefixLength);
-}
-
-static void emitNameUsingSpelling(raw_ostream , const Record ) {
-  size_t PrefixLength;
-  OS << "llvm::StringLiteral(";
-  write_cstring(
-  OS, StringRef(getOptionSpelling(R, PrefixLength)).substr(PrefixLength));
-  OS << ")";
+  return (Prefixes[0] + Twine(Name)).str();
 }
 
 class MarshallingInfo {
@@ -105,8 +89,6 @@
   }
 
   void emit(raw_ostream ) const {
-write_cstring(OS, StringRef(getOptionSpelling(R)));
-OS << ", ";
 OS << ShouldParse;
 OS << ", ";
 OS << ShouldAlwaysEmit;
@@ -303,7 +285,7 @@
 // The option prefix;
 OS << "llvm::ArrayRef()";
 
-// The option string.
+// The option prefixed name.
 OS << ", \"" << R.getValueAsString("Name") << '"';
 
 // The option identifier name.
@@ -346,8 +328,10 @@
 std::vector RPrefixes = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", ";
 
-// The option string.
-emitNameUsingSpelling(OS, R);
+// The option prefixed name.
+OS << "llvm::StringLiteral(";
+write_cstring(OS, getOptionPrefixedName(R));
+OS << ")";
 
 // The option identifier name.
 OS << ", " << getOptionName(R);
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 struct OptionWithMarshallingInfo {
-  llvm::StringRef Name;
+  llvm::StringLiteral PrefixedName;
   const char *KeyPath;
   const char *ImpliedCheck;
   const char *ImpliedValue;
@@ -18,20 +18,20 @@
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(   \
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
-HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+PREFIX_TYPE, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
+PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,  \
 DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
 MERGER, EXTRACTOR, TABLE_INDEX)\
-  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
+  {PREFIXED_NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
 
 TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
-  ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
-  ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
-  ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
-  ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
+  ASSERT_EQ(MarshallingTable[0].PrefixedName, "-marshalled-flag-d");
+  ASSERT_EQ(MarshallingTable[1].PrefixedName, "-marshalled-flag-c");
+  ASSERT_EQ(MarshallingTable[2].PrefixedName, "-marshalled-flag-b");
+  ASSERT_EQ(MarshallingTable[3].PrefixedName, "-marshalled-flag-a");
 }
 
 TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -59,7 +59,7 @@
   if ( == )
 return false;
 
-  if (int N = StrCmpOptionName(A.Name, B.Name))
+  if (int N = StrCmpOptionName(A.getName(), B.getName()))
 return N < 0;
 
   for (size_t I = 0, K = std::min(A.Prefixes.size(), 

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D157029#4561490 , @MaskRay wrote:

> This increases the size of  `Info` (static data size and static relocations). 
> In return, some dynamic relocations are saved. Is this a net win?

If that's a concern, I can remove `Info::Name` and replace its usages with a 
function call that drops the prefix from prefixed name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This increases the size of  `Info` (static data size and static relocations). 
In return, some dynamic relocations are saved. Is this a net win?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: llvm/include/llvm/Option/Option.h:103
 
+  StringLiteral getSpelling() const {
+assert(Info && "Must have a valid info!");

benlangmuir wrote:
> This could use a doc comment to differentiate it from other string 
> representations.
> 
> How does this compare with `Arg::getSpelling`? With `Arg`, IIUC the 
> "spelling" is how it was actually written rather than a canonical form.  That 
> might be confusing if this one is canonical; so we should at least clearly 
> document it or maybe put "canonical" in the API name?
You're right `Arg::getSpelling()` is the as-written prefix and name, while 
`Option::getSpelling()` was the canonical prefix and name. I noticed there's 
also `Option::getPrefixedName()` which used to return `std::string`. I decided 
to reuse that and return `StringLiteral` instead. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157029/new/

https://reviews.llvm.org/D157029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits