This revision was automatically updated to reflect the committed changes.
Closed by commit rL353761: [WebAssembly] Make thread-related options consistent
(authored by aheejin, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57874/new/
https://reviews.llvm.org/D57874
Files:
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
Index: cfe/trunk/test/Preprocessor/wasm-target-features.c
===================================================================
--- cfe/trunk/test/Preprocessor/wasm-target-features.c
+++ cfe/trunk/test/Preprocessor/wasm-target-features.c
@@ -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{{$}}
Index: cfe/trunk/test/Driver/wasm-toolchain.c
===================================================================
--- cfe/trunk/test/Driver/wasm-toolchain.c
+++ cfe/trunk/test/Driver/wasm-toolchain.c
@@ -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'
Index: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
===================================================================
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,27 @@
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 @@
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 @@
}
}
-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 {
Index: cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
===================================================================
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
+++ cfe/trunk/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: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/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: cfe/trunk/lib/Driver/Driver.cpp
===================================================================
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/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: cfe/trunk/include/clang/Driver/Options.td
===================================================================
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2160,8 +2160,6 @@
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]>,
Index: cfe/trunk/include/clang/Driver/ToolChain.h
===================================================================
--- cfe/trunk/include/clang/Driver/ToolChain.h
+++ cfe/trunk/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
