aheejin updated this revision to Diff 186065.
aheejin added a comment.

- Small fix


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c

Index: test/Driver/wasm-toolchain.c
===================================================================
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,35 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics 2>&1 | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR0 %s
+// THREAD_OPT_ERROR0: invalid argument '-matomics' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -no-pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR1 %s
+// THREAD_OPT_ERROR1: invalid argument '-matomics' not allowed with '-no-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR2 %s
+// THREAD_OPT_ERROR2: invalid argument '-mno-atomics' not allowed with '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR3 %s
+// THREAD_OPT_ERROR3: invalid argument '-mno-atomics' not allowed with '-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR4 %s
+// THREAD_OPT_ERROR4: invalid argument '-pthread' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -no-pthread -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR5 %s
+// THREAD_OPT_ERROR5: invalid argument '-no-pthread' not allowed with '-mthread-model posix'
Index: lib/Driver/ToolChains/WebAssembly.h
===================================================================
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
       llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===================================================================
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,65 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver &Driver, const ArgList &DriverArgs,
+                     bool &Atomics, bool &Pthread, StringRef &ThreadModel,
+                     bool CheckForErrors = true) {
+  // Default value for -matomics / -pthread / -mthread-model options, each being
+  // false / false / "single".
+  Atomics = DriverArgs.hasFlag(clang::driver::options::OPT_matomics,
+                               clang::driver::options::OPT_mno_atomics, false);
+  Pthread = DriverArgs.hasFlag(clang::driver::options::OPT_pthread,
+                               clang::driver::options::OPT_no_pthread, false);
+  ThreadModel = DriverArgs.getLastArgValue(
+      clang::driver::options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+    return;
+
+  // Error checking
+
+  // Did user explicitly specify -matomics / -mno-atomics / -mthread-model /
+  // -pthread / -no-pthread?
+  bool HasAtomics =
+      Atomics && DriverArgs.hasArg(clang::driver::options::OPT_matomics);
+  bool HasNoAtomics =
+      !Atomics && DriverArgs.hasArg(clang::driver::options::OPT_mno_atomics);
+  bool HasThreadModel =
+      DriverArgs.hasArg(clang::driver::options::OPT_mthread_model);
+  bool HasPthread =
+      Pthread && DriverArgs.hasArg(clang::driver::options::OPT_pthread);
+  bool HasNoPthread =
+      !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+
+  std::string ThreadModelOpt =
+      std::string("-mthread-model ") + ThreadModel.data();
+  // '-matomics' cannot be used with '-mthread-model single' (or anything not
+  // "posix")
+  if (HasAtomics && HasThreadModel && ThreadModel != "posix")
+    Driver.Diag(diag::err_drv_argument_not_allowed_with)
+        << "-matomics" << ThreadModelOpt;
+  // '-matomics' cannot be used with '-no-pthread'
+  if (HasAtomics && HasNoPthread)
+    Driver.Diag(diag::err_drv_argument_not_allowed_with) << "-matomics"
+                                                         << "-no-pthread";
+  // '-mno-atomics' cannot be used with '-mthread-model posix'
+  if (HasNoAtomics && HasThreadModel && ThreadModel == "posix")
+    Driver.Diag(diag::err_drv_argument_not_allowed_with)
+        << "-mno-atomics" << ThreadModelOpt;
+  // '-mno-atomics' cannot be used with '-pthread'
+  if (HasNoAtomics && HasPthread)
+    Driver.Diag(diag::err_drv_argument_not_allowed_with) << "-mno-atomics"
+                                                         << "-pthread";
+  // '-pthread' cannot be used with '-mthread-model single' (or anything not
+  // "posix")
+  if (HasPthread && HasThreadModel && ThreadModel != "posix")
+    Driver.Diag(diag::err_drv_argument_not_allowed_with)
+        << "-pthread" << ThreadModelOpt;
+  // '-no-pthread' cannot be used with '-mthread-model posix'
+  if (HasNoPthread && HasThreadModel && ThreadModel == "posix")
+    Driver.Diag(diag::err_drv_argument_not_allowed_with)
+        << "-no-pthread" << ThreadModelOpt;
+}
+
 wasm::Linker::Linker(const ToolChain &TC)
     : GnuTool("wasm::Linker", "lld", TC) {}
 
@@ -62,7 +121,9 @@
     if (ToolChain.ShouldLinkCXXStdlib(Args))
       ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
 
-    if (Args.hasArg(options::OPT_pthread))
+    if (Args.hasFlag(clang::driver::options::OPT_pthread,
+                     clang::driver::options::OPT_no_pthread, false),
+        false)
       CmdArgs.push_back("-lpthread");
 
     CmdArgs.push_back("-lc");
@@ -123,6 +184,14 @@
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
                          options::OPT_fno_use_init_array, true))
     CC1Args.push_back("-fuse-init-array");
+
+  bool Atomics = false, Pthread = false;
+  StringRef ThreadModel;
+  parseThreadArgs(getDriver(), DriverArgs, Atomics, Pthread, ThreadModel);
+  if (Pthread || ThreadModel == "posix") {
+    CC1Args.push_back("-target-feature");
+    CC1Args.push_back("+atomics");
+  }
 }
 
 ToolChain::RuntimeLibType WebAssembly::GetDefaultRuntimeLibType() const {
@@ -180,11 +249,15 @@
   }
 }
 
-std::string WebAssembly::getThreadModel() const {
-  // The WebAssembly MVP does not yet support threads; for now, use the
-  // "single" threading model, which lowers atomics to non-atomic operations.
-  // When threading support is standardized and implemented in popular engines,
-  // this override should be removed.
+std::string WebAssembly::getThreadModel(const ArgList &DriverArgs) const {
+  // The WebAssembly MVP does not yet support threads. We set this to "posix"
+  // only when we have atomics target feature on or '-pthread' is set.
+  bool Atomics = false, Pthread = false;
+  StringRef ThreadModel;
+  parseThreadArgs(getDriver(), DriverArgs, Atomics, Pthread, ThreadModel,
+                  false);
+  if (Atomics || Pthread)
+    return "posix";
   return "single";
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3823,7 +3823,7 @@
     CmdArgs.push_back(A->getValue());
   }
   else
-    CmdArgs.push_back(Args.MakeArgString(TC.getThreadModel()));
+    CmdArgs.push_back(Args.MakeArgString(TC.getThreadModel(Args)));
 
   Args.AddLastArg(CmdArgs, options::OPT_fveclib);
 
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1528,7 +1528,7 @@
     if (TC.isThreadModelSupported(A->getValue()))
       OS << "Thread model: " << A->getValue();
   } else
-    OS << "Thread model: " << TC.getThreadModel();
+    OS << "Thread model: " << TC.getThreadModel(C.getArgs());
   OS << '\n';
 
   // Print out the install directory.
Index: include/clang/Driver/ToolChain.h
===================================================================
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -453,7 +453,9 @@
   virtual bool SupportsEmbeddedBitcode() const { return false; }
 
   /// getThreadModel() - Which thread model does this target use?
-  virtual std::string getThreadModel() const { return "posix"; }
+  virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
+    return "posix";
+  }
 
   /// isThreadModelSupported() - Does this target support a thread model?
   virtual bool isThreadModelSupported(const StringRef Model) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to