r353761 - [WebAssembly] Make thread-related options consistent
Heejin Ahn via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 11 14:47:51 PST 2019
Author: aheejin
Date: Mon Feb 11 14:47:50 2019
New Revision: 353761
URL: http://llvm.org/viewvc/llvm-project?rev=353761&view=rev
Log:
[WebAssembly] Make thread-related options consistent
Summary:
There have been three options related to threads and users had to set
all three of them separately to get the correct compilation results.
This makes sure the relationship between the options makes sense and
sets necessary options for users if only part of the necessary options
are specified. This does:
- Remove `-matomics`; this option alone does not enable anything, so
removed it to not confuse users.
- `-mthread-model posix` sets `-target-feature +atomics`
- `-pthread` sets both `-target-feature +atomics` and
`-mthread-model posix`
Also errors out when explicitly given options don't match, such as
`-pthread` is given with `-mthread-model single`.
Reviewers: dschuff, sbc100, tlively, sunfish
Subscribers: jgravelle-google, jfb, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57874
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Preprocessor/wasm-target-features.c
Modified: cfe/trunk/include/clang/Driver/Options.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Feb 11 14:47:50 2019
@@ -2160,8 +2160,6 @@ def mexception_handing : Flag<["-"], "me
def mno_exception_handing : Flag<["-"], "mno-exception-handling">, Group<m_wasm_Features_Group>;
def mbulk_memory : Flag<["-"], "mbulk-memory">, Group<m_wasm_Features_Group>;
def mno_bulk_memory : Flag<["-"], "mno-bulk-memory">, Group<m_wasm_Features_Group>;
-def matomics : Flag<["-"], "matomics">, Group<m_wasm_Features_Group>;
-def mno_atomics : Flag<["-"], "mno-atomics">, Group<m_wasm_Features_Group>;
def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
Flags<[HelpHidden]>,
Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Mon Feb 11 14:47:50 2019
@@ -453,7 +453,9 @@ public:
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;
Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Mon Feb 11 14:47:50 2019
@@ -1528,7 +1528,7 @@ void Driver::PrintVersion(const Compilat
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.
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Feb 11 14:47:50 2019
@@ -3823,7 +3823,7 @@ void Clang::ConstructJob(Compilation &C,
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);
Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Mon Feb 11 14:47:50 2019
@@ -20,6 +20,27 @@ using namespace clang::driver::toolchain
using namespace clang;
using namespace llvm::opt;
+void parseThreadArgs(const Driver &Driver, const ArgList &DriverArgs,
+ bool &Pthread, StringRef &ThreadModel,
+ bool CheckForErrors = true) {
+ // Default value for -pthread / -mthread-model options, each being false /
+ // "single".
+ Pthread =
+ DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+ ThreadModel =
+ DriverArgs.getLastArgValue(options::OPT_mthread_model, "single");
+ if (!CheckForErrors)
+ return;
+
+ // Did user explicitly specify -mthread-model / -pthread?
+ bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+ bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
+ // '-pthread' cannot be used with '-mthread-model single'
+ if (HasPthread && HasThreadModel && ThreadModel == "single")
+ Driver.Diag(diag::err_drv_argument_not_allowed_with)
+ << "-pthread" << "-mthread-model single";
+}
+
wasm::Linker::Linker(const ToolChain &TC)
: GnuTool("wasm::Linker", "lld", TC) {}
@@ -123,6 +144,17 @@ void WebAssembly::addClangTargetOptions(
if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
options::OPT_fno_use_init_array, true))
CC1Args.push_back("-fuse-init-array");
+
+ // Either '-mthread-model posix' or '-pthread' sets '-target-feature
+ // +atomics'. We intentionally didn't create '-matomics' and set the atomics
+ // target feature here depending on the other two options.
+ bool Pthread = false;
+ StringRef ThreadModel = "";
+ parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel);
+ if (Pthread || ThreadModel != "single") {
+ CC1Args.push_back("-target-feature");
+ CC1Args.push_back("+atomics");
+ }
}
ToolChain::RuntimeLibType WebAssembly::GetDefaultRuntimeLibType() const {
@@ -180,12 +212,15 @@ void WebAssembly::AddCXXStdlibLibArgs(co
}
}
-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.
- return "single";
+std::string WebAssembly::getThreadModel(const ArgList &DriverArgs) const {
+ // The WebAssembly MVP does not yet support threads. We set this to "posix"
+ // when '-pthread' is set.
+ bool Pthread = false;
+ StringRef ThreadModel = "";
+ parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel, false);
+ if (Pthread)
+ return "posix";
+ return ThreadModel;
}
Tool *WebAssembly::buildLinker() const {
Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.h?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.h Mon Feb 11 14:47:50 2019
@@ -64,7 +64,7 @@ private:
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"; }
Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Mon Feb 11 14:47:50 2019
@@ -38,3 +38,16 @@
// 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.
+// - '-mthread-model' sets '-target-feature +matomics'
+// - '-phread' sets both '-target-featuer +atomics' and '-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 -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR %s
+// THREAD_OPT_ERROR: invalid argument '-pthread' not allowed with '-mthread-model single'
Modified: cfe/trunk/test/Preprocessor/wasm-target-features.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/wasm-target-features.c?rev=353761&r1=353760&r2=353761&view=diff
==============================================================================
--- cfe/trunk/test/Preprocessor/wasm-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/wasm-target-features.c Mon Feb 11 14:47:50 2019
@@ -52,11 +52,13 @@
//
// BULK-MEMORY:#define __wasm_bulk_memory__ 1{{$}}
//
+// We don't use -matomics directly and '-mthread-model posix' sets the atomics
+// target feature.
// RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm32-unknown-unknown -matomics \
+// RUN: -target wasm32-unknown-unknown -mthread-model posix \
// RUN: | FileCheck %s -check-prefix=ATOMICS
// RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm64-unknown-unknown -matomics \
+// RUN: -target wasm64-unknown-unknown -mthread-model posix \
// RUN: | FileCheck %s -check-prefix=ATOMICS
//
// ATOMICS:#define __wasm_atomics__ 1{{$}}
More information about the cfe-commits
mailing list