Allow multiple invocations with arguments for all passes, allow --pas…

…s-arg only for additional arguments.
WebAssembly · Jun 25, 2024 · a12f18b · a12f18b
1 parent d74f5b1
commit a12f18b
Show file tree

Hide file tree

Showing 20 changed files with 92 additions and 99 deletions.
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
@@ -5451,7 +5451,9 @@ void BinaryenModuleRunPasses(BinaryenModuleRef module,
  PassRunner passRunner((Module*)module);
  passRunner.options = globalPassOptions;
  for (BinaryenIndex i = 0; i < numPasses; i++) {
- passRunner.add(passes[i]);
+ passRunner.add(passes[i]), globalPassOptions.arguments.count(passes[i]) > 0
+ ? globalPassOptions.arguments[passes[i]]
+ : std::optional<std::string>();
  }
  passRunner.run();
 }
@@ -5696,7 +5698,10 @@ void BinaryenFunctionRunPasses(BinaryenFunctionRef func,
  PassRunner passRunner((Module*)module);
  passRunner.options = globalPassOptions;
  for (BinaryenIndex i = 0; i < numPasses; i++) {
- passRunner.add(passes[i]);
+ passRunner.add(passes[i],
+ globalPassOptions.arguments.count(passes[i]) > 0
+ ? globalPassOptions.arguments[passes[i]]
+ : std::optional<std::string>());
  }
  passRunner.runOnFunction((Function*)func);
 }

diff --git a/src/pass.h b/src/pass.h
@@ -38,10 +38,7 @@ struct PassRegistry {
 
  using Creator = std::function<Pass*()>;
 
- void registerPass(const char* name,
- const char* description,
- Creator create,
- bool allowMultipleInstancesWithArgs = false);
+ void registerPass(const char* name, const char* description, Creator create);
  ;
  // Register a pass that's used for internal testing. These passes do not show
  // up in --help.
@@ -51,7 +48,6 @@ struct PassRegistry {
  std::vector<std::string> getRegisteredNames();
  std::string getPassDescription(std::string name);
  bool isPassHidden(std::string name);
- bool doesPassAllowMultipleInstancesWithArgs(std::string name);
 
 private:
  void registerPasses();
@@ -60,14 +56,9 @@ struct PassRegistry {
  std::string description;
  Creator create;
  bool hidden;
- bool allowMultipleInstancesWithArgs;
  PassInfo() = default;
- PassInfo(std::string description,
- Creator create,
- bool hidden = false,
- bool allowMultipleInstancesWithArgs = false)
- : description(description), create(create), hidden(hidden),
- allowMultipleInstancesWithArgs(allowMultipleInstancesWithArgs) {}
+ PassInfo(std::string description, Creator create, bool hidden = false)
+ : description(description), create(create), hidden(hidden) {}
  };
  std::map<std::string, PassInfo> passInfos;
 };
@@ -495,7 +486,7 @@ class Pass {
  // to imports must override this to return true.
  virtual bool addsEffects() { return false; }
 
- void setPassArg(std::string value) { passArg = value; }
+ void setPassArg(const std::string& value) { passArg = value; }
 
  std::string name;
 
@@ -508,6 +499,12 @@ class Pass {
  PassOptions& getPassOptions() { return runner->options; }
 
 protected:
+ bool hasArgument(const std::string& key);
+ std::string getArgument(const std::string& key,
+ const std::string& errorTextIfMissing);
+ std::string getArgumentOrDefault(const std::string& key,
+ const std::string& defaultValue);
+
  std::optional<std::string> passArg;
 
  Pass() = default;

diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp
@@ -1608,54 +1608,49 @@ struct Asyncify : public Pass {
  bool addsEffects() override { return true; }
 
  void run(Module* module) override {
- auto& options = getPassOptions();
- bool optimize = options.optimizeLevel > 0;
+ bool optimize = getPassOptions().optimizeLevel > 0;
 
  // Find which things can change the state.
  auto stateChangingImports = String::trim(read_possible_response_file(
- options.getArgumentOrDefault("asyncify-imports", "")));
- auto ignoreImports =
- options.getArgumentOrDefault("asyncify-ignore-imports", "");
+ getArgumentOrDefault("asyncify-imports", "")));
+ auto ignoreImports = getArgumentOrDefault("asyncify-ignore-imports", "");
  bool allImportsCanChangeState =
  stateChangingImports == "" && ignoreImports == "";
  String::Split listedImports(stateChangingImports,
  String::Split::NewLineOr(","));
  // canIndirectChangeState is the default. asyncify-ignore-indirect sets it
  // to false.
- auto canIndirectChangeState =
- !options.hasArgument("asyncify-ignore-indirect");
+ auto canIndirectChangeState = !hasArgument("asyncify-ignore-indirect");
  std::string removeListInput =
- options.getArgumentOrDefault("asyncify-removelist", "");
+ getArgumentOrDefault("asyncify-removelist", "");
  if (removeListInput.empty()) {
  // Support old name for now to avoid immediate breakage TODO remove
- removeListInput = options.getArgumentOrDefault("asyncify-blacklist", "");
+ removeListInput = getArgumentOrDefault("asyncify-blacklist", "");
  }
  String::Split removeList(
  String::trim(read_possible_response_file(removeListInput)),
  String::Split::NewLineOr(","));
- String::Split addList(
- String::trim(read_possible_response_file(
- options.getArgumentOrDefault("asyncify-addlist", ""))),
- String::Split::NewLineOr(","));
- std::string onlyListInput =
- options.getArgumentOrDefault("asyncify-onlylist", "");
+ String::Split addList(String::trim(read_possible_response_file(
+ getArgumentOrDefault("asyncify-addlist", ""))),
+ String::Split::NewLineOr(","));
+ std::string onlyListInput = getArgumentOrDefault("asyncify-onlylist", "");
  if (onlyListInput.empty()) {
  // Support old name for now to avoid immediate breakage TODO remove
- onlyListInput = options.getArgumentOrDefault("asyncify-whitelist", "");
+ onlyListInput = getArgumentOrDefault("asyncify-whitelist", "");
  }
  String::Split onlyList(
  String::trim(read_possible_response_file(onlyListInput)),
  String::Split::NewLineOr(","));
- auto asserts = options.hasArgument("asyncify-asserts");
- auto verbose = options.hasArgument("asyncify-verbose");
- auto relocatable = options.hasArgument("asyncify-relocatable");
- auto secondaryMemory = options.hasArgument("asyncify-in-secondary-memory");
- auto propagateAddList = options.hasArgument("asyncify-propagate-addlist");
+ auto asserts = hasArgument("asyncify-asserts");
+ auto verbose = hasArgument("asyncify-verbose");
+ auto relocatable = hasArgument("asyncify-relocatable");
+ auto secondaryMemory = hasArgument("asyncify-in-secondary-memory");
+ auto propagateAddList = hasArgument("asyncify-propagate-addlist");
 
  // Ensure there is a memory, as we need it.
  if (secondaryMemory) {
  auto secondaryMemorySizeString =
- options.getArgumentOrDefault("asyncify-secondary-memory-size", "1");
+ getArgumentOrDefault("asyncify-secondary-memory-size", "1");
  Address secondaryMemorySize = std::stoi(secondaryMemorySizeString);
  asyncifyMemory = createSecondaryMemory(module, secondaryMemorySize);
  } else {

diff --git a/src/passes/Directize.cpp b/src/passes/Directize.cpp
@@ -203,7 +203,7 @@ struct Directize : public Pass {
 
  // TODO: consider a per-table option here
  auto initialContentsImmutable =
- getPassOptions().hasArgument("directize-initial-contents-immutable");
+ hasArgument("directize-initial-contents-immutable");
 
  // Set up the initial info.
  TableInfoMap tables;

diff --git a/src/passes/ExtractFunction.cpp b/src/passes/ExtractFunction.cpp
@@ -62,7 +62,7 @@ struct ExtractFunction : public Pass {
  bool addsEffects() override { return true; }
 
  void run(Module* module) override {
- Name name = getPassOptions().getArgument(
+ Name name = getArgument(
  "extract-function",
  "ExtractFunction usage: wasm-opt --extract-function=FUNCTION_NAME");
  extract(getPassRunner(), module, name);
@@ -74,10 +74,9 @@ struct ExtractFunctionIndex : public Pass {
  bool addsEffects() override { return true; }
 
  void run(Module* module) override {
- std::string index =
- getPassOptions().getArgument("extract-function-index",
- "ExtractFunctionIndex usage: wasm-opt "
- "--extract-function-index=FUNCTION_INDEX");
+ std::string index = getArgument("extract-function-index",
+ "ExtractFunctionIndex usage: wasm-opt "
+ "--extract-function-index=FUNCTION_INDEX");
  for (char c : index) {
  if (!std::isdigit(c)) {
  Fatal() << "Expected numeric function index";

diff --git a/src/passes/FuncCastEmulation.cpp b/src/passes/FuncCastEmulation.cpp
@@ -157,8 +157,7 @@ struct FuncCastEmulation : public Pass {
  bool addsEffects() override { return true; }
 
  void run(Module* module) override {
- Index numParams = std::stoul(
- getPassOptions().getArgumentOrDefault("max-func-params", "16"));
+ Index numParams = std::stoul(getArgumentOrDefault("max-func-params", "16"));
  // we just need the one ABI function type for all indirect calls
  HeapType ABIType(
  Signature(Type(std::vector<Type>(numParams, Type::i64)), Type::i64));

diff --git a/src/passes/JSPI.cpp b/src/passes/JSPI.cpp
@@ -83,18 +83,17 @@ struct JSPI : public Pass {
  void run(Module* module) override {
  Builder builder(*module);
 
- auto& options = getPassOptions();
  // Find which imports can suspend.
- auto stateChangingImports = String::trim(read_possible_response_file(
- options.getArgumentOrDefault("jspi-imports", "")));
+ auto stateChangingImports = String::trim(
+ read_possible_response_file(getArgumentOrDefault("jspi-imports", "")));
  String::Split listedImports(stateChangingImports, ",");
 
  // Find which exports should create a promise.
- auto stateChangingExports = String::trim(read_possible_response_file(
- options.getArgumentOrDefault("jspi-exports", "")));
+ auto stateChangingExports = String::trim(
+ read_possible_response_file(getArgumentOrDefault("jspi-exports", "")));
  String::Split listedExports(stateChangingExports, ",");
 
- bool wasmSplit = options.hasArgument("jspi-split-module");
+ bool wasmSplit = hasArgument("jspi-split-module");
  if (wasmSplit) {
  // Make an import for the load secondary module function so a JSPI wrapper
  // version will be created.

diff --git a/src/passes/LegalizeJSInterface.cpp b/src/passes/LegalizeJSInterface.cpp
@@ -64,9 +64,8 @@ struct LegalizeJSInterface : public Pass {
  setTempRet0 = nullptr;
  getTempRet0 = nullptr;
  auto exportOriginals =
- getPassOptions().hasArgument("legalize-js-interface-export-originals");
- exportedHelpers =
- getPassOptions().hasArgument("legalize-js-interface-exported-helpers");
+ hasArgument("legalize-js-interface-export-originals");
+ exportedHelpers = hasArgument("legalize-js-interface-exported-helpers");
  // for each illegal export, we must export a legalized stub instead
  std::vector<std::unique_ptr<Export>> newExports;
  for (auto& ex : module->exports) {

diff --git a/src/passes/LogExecution.cpp b/src/passes/LogExecution.cpp
@@ -46,8 +46,7 @@ struct LogExecution : public WalkerPass<PostWalker<LogExecution>> {
  bool addsEffects() override { return true; }
 
  void run(Module* module) override {
- auto& options = getPassOptions();
- loggerModule = options.getArgumentOrDefault("log-execution", "");
+ loggerModule = getArgumentOrDefault("log-execution", "");
  super::run(module);
  }
 

diff --git a/src/passes/NoInline.cpp b/src/passes/NoInline.cpp
@@ -49,9 +49,7 @@ struct NoInline : public Pass {
 
  void run(Module* module) override {
  std::string pattern =
- passArg ? *passArg
- : getPassOptions().getArgument(
- name, "Usage usage: wasm-opt --" + name + "=WILDCARD");
+ getArgument(name, "Usage usage: wasm-opt --" + name + "=WILDCARD");
 
  for (auto& func : module->functions) {
  if (!String::wildcardMatch(pattern, func->name.toString())) {

diff --git a/src/passes/PostEmscripten.cpp b/src/passes/PostEmscripten.cpp
@@ -214,8 +214,7 @@ struct PostEmscripten : public Pass {
  std::vector<Address> segmentOffsets; // segment index => address offset
  calcSegmentOffsets(module, segmentOffsets);
 
- auto& options = getPassOptions();
- auto sideModule = options.hasArgument("post-emscripten-side-module");
+ auto sideModule = hasArgument("post-emscripten-side-module");
  if (!sideModule) {
  removeData(module, segmentOffsets, "__start_em_asm", "__stop_em_asm");
  removeData(module, segmentOffsets, "__start_em_js", "__stop_em_js");
@@ -235,8 +234,7 @@ struct PostEmscripten : public Pass {
  }
 
  void removeEmJsExports(Module& module) {
- auto& options = getPassOptions();
- auto sideModule = options.hasArgument("post-emscripten-side-module");
+ auto sideModule = hasArgument("post-emscripten-side-module");
  EmJsWalker walker(sideModule);
  walker.walkModule(&module);
  for (const Export& exp : walker.toRemove) {

diff --git a/src/passes/PrintFunctionMap.cpp b/src/passes/PrintFunctionMap.cpp
@@ -37,7 +37,7 @@ struct PrintFunctionMap : public Pass {
  void run(Module* module) override {
  // If an argument is provided, write to that file; otherwise write to
  // stdout.
- auto outFile = getPassOptions().getArgumentOrDefault("symbolmap", "");
+ auto outFile = getArgumentOrDefault("symbolmap", "");
  Output output(outFile, Flags::Text);
  auto& o = output.getStream();
  Index i = 0;

diff --git a/src/passes/SeparateDataSegments.cpp b/src/passes/SeparateDataSegments.cpp
@@ -33,14 +33,14 @@ struct SeparateDataSegments : public Pass {
 
  void run(Module* module) override {
  std::string outfileName =
- getPassOptions().getArgument("separate-data-segments",
-  "SeparateDataSegments usage: wasm-opt "
-  "--separate-data-segments@FILENAME");
+ getArgument("separate-data-segments",
+ "SeparateDataSegments usage: wasm-opt "
+ "--separate-data-segments@FILENAME");
  Output outfile(outfileName, Flags::Binary);
- std::string baseStr = getPassOptions().getArgument(
- "separate-data-segments-global-base",
- "SeparateDataSegments usage: wasm-opt "
- "--pass-arg=separate-data-segments-global-base@NUMBER");
+ std::string baseStr =
+ getArgument("separate-data-segments-global-base",
+  "SeparateDataSegments usage: wasm-opt "
+  "--pass-arg=separate-data-segments-global-base@NUMBER");
  Address base = std::stoi(baseStr);
  size_t lastEnd = 0;
  for (auto& seg : module->dataSegments) {

diff --git a/src/passes/SetGlobals.cpp b/src/passes/SetGlobals.cpp
@@ -29,9 +29,9 @@ struct SetGlobals : public Pass {
  bool requiresNonNullableLocalFixups() override { return false; }
 
  void run(Module* module) override {
- Name input = getPassRunner()->options.getArgument(
- "set-globals",
- "SetGlobals usage: wasm-opt --pass-arg=set-globals@x=y,z=w");
+ Name input =
+ getArgument("set-globals",
+  "SetGlobals usage: wasm-opt --pass-arg=set-globals@x=y,z=w");
 
  // The input is a set of X=Y pairs separated by commas.
  String::Split pairs(input.toString(), ",");

diff --git a/src/passes/StackCheck.cpp b/src/passes/StackCheck.cpp
@@ -141,8 +141,7 @@ struct StackCheck : public Pass {
  auto stackLimitName = Names::getValidGlobalName(*module, "__stack_limit");
 
  Name handler;
- auto handlerName =
- getPassOptions().getArgumentOrDefault("stack-check-handler", "");
+ auto handlerName = getArgumentOrDefault("stack-check-handler", "");
  if (handlerName != "") {
  handler = handlerName;
  importStackOverflowHandler(

diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp
@@ -43,11 +43,9 @@ PassRegistry* PassRegistry::get() { return &singleton; }
 
 void PassRegistry::registerPass(const char* name,
  const char* description,
- Creator create,
- bool allowMultipleInstancesWithArgs) {
+ Creator create) {
  assert(passInfos.find(name) == passInfos.end());
- passInfos[name] =
- PassInfo(description, create, false, allowMultipleInstancesWithArgs);
+ passInfos[name] = PassInfo(description, create, false);
 }
 
 void PassRegistry::registerTestPass(const char* name,
@@ -85,12 +83,6 @@ bool PassRegistry::isPassHidden(std::string name) {
  return passInfos[name].hidden;
 }
 
-bool PassRegistry::doesPassAllowMultipleInstancesWithArgs(std::string name) {
- assert(passInfos.find(name) != passInfos.end());
-
- return passInfos[name].allowMultipleInstancesWithArgs;
-}
-
 // PassRunner
 
 void PassRegistry::registerPasses() {
@@ -308,8 +300,7 @@ void PassRegistry::registerPasses() {
  createMultiMemoryLoweringWithBoundsChecksPass);
  registerPass("nm", "name list", createNameListPass);
  registerPass("name-types", "(re)name all heap types", createNameTypesPass);
- registerPass(
- "no-inline", "mark functions as no-inline", createNoInlinePass, true);
+ registerPass("no-inline", "mark functions as no-inline", createNoInlinePass);
  registerPass("no-full-inline",
  "mark functions as no-inline (for full inlining only)",
  createNoFullInlinePass);
@@ -1036,4 +1027,28 @@ bool PassRunner::shouldPreserveDWARF() {
  return true;
 }
 
+bool Pass::hasArgument(const std::string& key) {
+ return (key == name) ? passArg.has_value()
+ : getPassOptions().hasArgument(key);
+}
+
+std::string Pass::getArgument(const std::string& key,
+ const std::string& errorTextIfMissing) {
+ if (!hasArgument(key)) {
+ Fatal() << errorTextIfMissing;
+ }
+
+ return (key == name) ? *passArg
+ : getPassOptions().getArgument(key, errorTextIfMissing);
+}
+
+std::string Pass::getArgumentOrDefault(const std::string& key,
+ const std::string& defaultValue) {
+ if (key == name) {
+ return passArg.value_or(defaultValue);
+ }
+
+ return getPassOptions().getArgumentOrDefault(key, defaultValue);
+}
+
 } // namespace wasm