[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29
+  Pthread =
+      DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
----------------
Shouldn't every use of `hasFlag` be `getLastArgValue` instead?


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:36
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
----------------
It doesn't matter whether the user included the -pthread flag if they later included the -no-pthread flag.


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:42
+  // "posix")
+  if (HasPthread && HasThreadModel && ThreadModel != "posix")
+    Driver.Diag(diag::err_drv_argument_not_allowed_with)
----------------
This should be `ThreadModel == "single"`, since the distinction we care about is single-threaded vs no single-threaded. If in the future there are dozens of thread models besides "posix" this logic should still work.


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:157
+  parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel);
+  if (Pthread || ThreadModel == "posix") {
+    CC1Args.push_back("-target-feature");
----------------
Same here. Should compare ThreadModel with "single".


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874