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

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D57874#1389981 <https://reviews.llvm.org/D57874#1389981>, @sunfish wrote:

> > - `-matomics` means `-mthread-model posix`
>
> The others sound reasonable, though this one seems a little surprising -- a user might have -matomics enabled because they're targeting a VM that has atomics, but still not want to use -mthread-model posix because their code doesn't actually using threads.


FYI, we don't have `-matomics` anymore.



================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+      !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+
----------------
sbc100 wrote:
> tlively wrote:
> > Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to check only that the final requested configuration is consistent rather than checking all intermediate configurations?
> Can you remove all the "clang::driver" namspace qualification here since there is  a "using" above?
@sbc100 Yeah good catch.


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+      !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+
----------------
aheejin wrote:
> sbc100 wrote:
> > tlively wrote:
> > > Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to check only that the final requested configuration is consistent rather than checking all intermediate configurations?
> > Can you remove all the "clang::driver" namspace qualification here since there is  a "using" above?
> @sbc100 Yeah good catch.
@tlively I used `getLastArgValue` when I get the thread model string above:
```
ThreadModel = DriverArgs.getLastArgValue(
      clang::driver::options::OPT_mthread_model, "single");
```
This takes the last occurrence of this argument, and the second argument is the default value when the user didn't specify that option.

Here I used `hasArg` just to determine whether the user gave it explicitly or not, because we error out only when a user explicitly gives conflicting set of options.


================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:62
+    Driver.Diag(diag::err_drv_argument_not_allowed_with) << "-matomics"
+                                                         << "-no-pthread";
+  // '-mno-atomics' cannot be used with '-mthread-model posix'
----------------
tlively wrote:
> I'm not sure about this one, either. What if I want atomics for multithreading but I don't want to link with libpthread?
Yeah good catch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874