[Mlir-commits] [mlir] a4deb14 - [LLVM][Support] Support for `llvm::cl::list`'s default values

Son Tuan Vu llvmlistbot at llvm.org
Thu Oct 6 10:50:54 PDT 2022


Author: Son Tuan Vu
Date: 2022-10-06T17:50:40Z
New Revision: a4deb14fdfaa183da938e40b54954afa9c7a11c1

URL: https://github.com/llvm/llvm-project/commit/a4deb14fdfaa183da938e40b54954afa9c7a11c1
DIFF: https://github.com/llvm/llvm-project/commit/a4deb14fdfaa183da938e40b54954afa9c7a11c1.diff

LOG: [LLVM][Support] Support for `llvm::cl::list`'s default values

This patch introduces support for default values of list of CL options.
It fixes the issue in https://github.com/llvm/llvm-project/issues/52667

Reviewed By: bkramer

Differential Revision: https://reviews.llvm.org/D135311

Added: 
    

Modified: 
    llvm/include/llvm/Support/CommandLine.h
    llvm/unittests/Support/CommandLineTest.cpp
    mlir/include/mlir/Pass/PassOptions.h
    mlir/lib/Pass/PassRegistry.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index 15deb8a0b027a..0788308577f59 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -437,10 +437,22 @@ template <class Ty> struct initializer {
   template <class Opt> void apply(Opt &O) const { O.setInitialValue(Init); }
 };
 
+template <class Ty> struct list_initializer {
+  ArrayRef<Ty> Inits;
+  list_initializer(ArrayRef<Ty> Vals) : Inits(Vals) {}
+
+  template <class Opt> void apply(Opt &O) const { O.setInitialValues(Inits); }
+};
+
 template <class Ty> initializer<Ty> init(const Ty &Val) {
   return initializer<Ty>(Val);
 }
 
+template <class Ty>
+list_initializer<Ty> list_init(ArrayRef<Ty> Vals) {
+  return list_initializer<Ty>(Vals);
+}
+
 // Allow the user to specify which external variable they want to store the
 // results of the command line argument processing into, if they don't want to
 // store it in the option itself.
@@ -1504,6 +1516,9 @@ extern template class opt<bool>;
 //
 template <class DataType, class StorageClass> class list_storage {
   StorageClass *Location = nullptr; // Where to store the object...
+  std::vector<OptionValue<DataType>> Default =
+      std::vector<OptionValue<DataType>>();
+  bool DefaultAssigned = false;
 
 public:
   list_storage() = default;
@@ -1517,12 +1532,22 @@ template <class DataType, class StorageClass> class list_storage {
     return false;
   }
 
-  template <class T> void addValue(const T &V) {
+  template <class T> void addValue(const T &V, bool initial = false) {
     assert(Location != nullptr &&
            "cl::location(...) not specified for a command "
            "line option with external storage!");
     Location->push_back(V);
+    if (initial)
+      Default.push_back(V);
+  }
+
+  const std::vector<OptionValue<DataType>> &getDefault() const {
+    return Default;
   }
+
+  void assignDefault() { DefaultAssigned = true; }
+  void overwriteDefault() { DefaultAssigned = false; }
+  bool isDefaultAssigned() { return DefaultAssigned; }
 };
 
 // Define how to hold a class type object, such as a string.
@@ -1535,6 +1560,8 @@ template <class DataType, class StorageClass> class list_storage {
 //
 template <class DataType> class list_storage<DataType, bool> {
   std::vector<DataType> Storage;
+  std::vector<OptionValue<DataType>> Default;
+  bool DefaultAssigned = false;
 
 public:
   using iterator = typename std::vector<DataType>::iterator;
@@ -1598,7 +1625,19 @@ template <class DataType> class list_storage<DataType, bool> {
   std::vector<DataType> *operator&() { return &Storage; }
   const std::vector<DataType> *operator&() const { return &Storage; }
 
-  template <class T> void addValue(const T &V) { Storage.push_back(V); }
+  template <class T> void addValue(const T &V, bool initial = false) {
+    Storage.push_back(V);
+    if (initial)
+      Default.push_back(OptionValue<DataType>(V));
+  }
+
+  const std::vector<OptionValue<DataType>> &getDefault() const {
+    return Default;
+  }
+
+  void assignDefault() { DefaultAssigned = true; }
+  void overwriteDefault() { DefaultAssigned = false; }
+  bool isDefaultAssigned() { return DefaultAssigned; }
 };
 
 //===----------------------------------------------------------------------===//
@@ -1622,6 +1661,10 @@ class list : public Option, public list_storage<DataType, StorageClass> {
                         StringRef Arg) override {
     typename ParserClass::parser_data_type Val =
         typename ParserClass::parser_data_type();
+    if (list_storage<DataType, StorageClass>::isDefaultAssigned()) {
+      clear();
+      list_storage<DataType, StorageClass>::overwriteDefault();
+    }
     if (Parser.parse(*this, ArgName, Arg, Val))
       return true; // Parse Error!
     list_storage<DataType, StorageClass>::addValue(Val);
@@ -1647,6 +1690,8 @@ class list : public Option, public list_storage<DataType, StorageClass> {
   void setDefault() override {
     Positions.clear();
     list_storage<DataType, StorageClass>::clear();
+    for (auto &Val : list_storage<DataType, StorageClass>::getDefault())
+      list_storage<DataType, StorageClass>::addValue(Val.getValue());
   }
 
   void done() {
@@ -1666,6 +1711,20 @@ class list : public Option, public list_storage<DataType, StorageClass> {
     return Positions[optnum];
   }
 
+  void clear() {
+    Positions.clear();
+    list_storage<DataType, StorageClass>::clear();
+  }
+
+  // setInitialValues - Used by the cl::list_init modifier...
+  void setInitialValues(ArrayRef<DataType> Vs) {
+    assert(!(list_storage<DataType, StorageClass>::isDefaultAssigned()) &&
+           "Cannot have two default values");
+    list_storage<DataType, StorageClass>::assignDefault();
+    for (auto &Val : Vs)
+      list_storage<DataType, StorageClass>::addValue(Val, true);
+  }
+
   void setNumAdditionalVals(unsigned n) { Option::setNumAdditionalVals(n); }
 
   template <class... Mods>

diff  --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 7af8c89e90e0f..dd6e12223dacb 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1038,7 +1038,7 @@ TEST(CommandLineTest, ResponseFileEOLs) {
   }
 }
 
-TEST(CommandLineTest, SetDefautValue) {
+TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
   StackOption<std::string> Opt1("opt1", cl::init("true"));
@@ -1046,15 +1046,32 @@ TEST(CommandLineTest, SetDefautValue) {
   cl::alias Alias("alias", llvm::cl::aliasopt(Opt2));
   StackOption<int> Opt3("opt3", cl::init(3));
 
-  const char *args[] = {"prog", "-opt1=false", "-opt2", "-opt3"};
+  llvm::SmallVector<int, 3> IntVals = {1, 2, 3};
+  llvm::SmallVector<std::string, 3> StrVals = {"foo", "bar", "baz"};
+
+  StackOption<int, cl::list<int>> List1(
+      "list1", cl::list_init<int>(llvm::ArrayRef<int>(IntVals)),
+      cl::CommaSeparated);
+  StackOption<std::string, cl::list<std::string>> List2(
+      "list2", cl::list_init<std::string>(llvm::ArrayRef<std::string>(StrVals)),
+      cl::CommaSeparated);
+  cl::alias ListAlias("list-alias", llvm::cl::aliasopt(List2));
+
+  const char *args[] = {"prog",   "-opt1=false", "-list1", "4",
+                        "-list1", "5,6",         "-opt2",  "-opt3"};
 
   EXPECT_TRUE(
-    cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
+      cl::ParseCommandLineOptions(7, args, StringRef(), &llvm::nulls()));
 
   EXPECT_EQ(Opt1, "false");
   EXPECT_TRUE(Opt2);
   EXPECT_EQ(Opt3, 3);
 
+  for (size_t I = 0, E = IntVals.size(); I < E; ++I) {
+    EXPECT_EQ(IntVals[I] + 3, List1[I]);
+    EXPECT_EQ(StrVals[I], List2[I]);
+  }
+
   Opt2 = false;
   Opt3 = 1;
 
@@ -1071,7 +1088,13 @@ TEST(CommandLineTest, SetDefautValue) {
   EXPECT_EQ(Opt1, "true");
   EXPECT_TRUE(Opt2);
   EXPECT_EQ(Opt3, 3);
+  for (size_t I = 0, E = IntVals.size(); I < E; ++I) {
+    EXPECT_EQ(IntVals[I], List1[I]);
+    EXPECT_EQ(StrVals[I], List2[I]);
+  }
+
   Alias.removeArgument();
+  ListAlias.removeArgument();
 }
 
 TEST(CommandLineTest, ReadConfigFile) {

diff  --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h
index 3ee91c7d463be..67c785c3633ce 100644
--- a/mlir/include/mlir/Pass/PassOptions.h
+++ b/mlir/include/mlir/Pass/PassOptions.h
@@ -229,6 +229,10 @@ class PassOptions : protected llvm::cl::SubCommand {
 
     bool handleOccurrence(unsigned pos, StringRef argName,
                           StringRef arg) override {
+      if (this->isDefaultAssigned()) {
+        this->clear();
+        this->overwriteDefault();
+      }
       this->optHasValue = true;
       return failed(detail::pass_options::parseCommaSeparatedList(
           *this, argName, arg, elementParser,
@@ -418,6 +422,7 @@ struct OptionValue<mlir::OpPassManager> final : GenericOptionValue {
   using WrapperType = mlir::OpPassManager;
 
   OptionValue();
+  OptionValue(const OptionValue<mlir::OpPassManager> &rhs);
   OptionValue(const mlir::OpPassManager &value);
   OptionValue<mlir::OpPassManager> &operator=(const mlir::OpPassManager &rhs);
   ~OptionValue();

diff  --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp
index 9f0ce23c5b817..2d6f3417b2c9e 100644
--- a/mlir/lib/Pass/PassRegistry.cpp
+++ b/mlir/lib/Pass/PassRegistry.cpp
@@ -348,6 +348,11 @@ llvm::cl::OptionValue<OpPassManager>::OptionValue(
     const mlir::OpPassManager &value) {
   setValue(value);
 }
+llvm::cl::OptionValue<OpPassManager>::OptionValue(
+    const llvm::cl::OptionValue<mlir::OpPassManager> &rhs) {
+  if (rhs.hasValue())
+    setValue(rhs.getValue());
+}
 llvm::cl::OptionValue<OpPassManager> &
 llvm::cl::OptionValue<OpPassManager>::operator=(
     const mlir::OpPassManager &rhs) {


        


More information about the Mlir-commits mailing list