aheejin updated this revision to Diff 186063.
aheejin added a comment.
- Fix some bugs
- Make the driver error out when explicitly given options don't match, such as
`-no-pthread` and `-matomics` are given at the same time
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)
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
