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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits