tlively marked an inline comment as done.
tlively added a comment.
In D58742#1413077 <https://reviews.llvm.org/D58742#1413077>, @sunfish wrote:
> This is still a little confusing to me. -matomic is supposed to be a
> subtarget flag, stating that the wasm implementation we will run on supports
> atomic instructions. -mthread-model posix is about the C++ interpretation --
> what style implementation of memory model do we want? In the future, -matomic
> may become enabled by default, when enough wasm engines generally support it.
> However, -mthread-model single/posix may still be useful to control
> independently, because even with wasm engines supporting atomic, there are
> reasons users might still want to compile their apps single-threaded: access
> to linear memory with no declared max, lower overall code size, or other
> things.
The only backend besides us that uses the ThreadModel at all is ARM, so we were
questioning its generality. I originally shared your opinion that the available
target features should be controlled separately from whether or not the user
wants threads, but in reality we get to use the atomics feature if and only if
the user opts in to threads. Because of this, ignoring the thread model
entirely like most other backends lets us have a much simpler user interface
that is not invalid by default.
I expect with this model we would never turn on atomics by default (since we
don't want multithreaded by default), and most users would get them by using
-pthread. This is very close in UI to native targets. The -matomics option is
only there for users who are doing weird threaded things but don't want
libpthread for some reason.
================
Comment at: clang/include/clang/Driver/ToolChain.h:456
/// getThreadModel() - Which thread model does this target use?
- virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
- return "posix";
- }
+ virtual std::string getThreadModel() const { return "posix"; }
----------------
aheejin wrote:
> I think now we can delete this overridden method, because the default value
> for this is "posix". See Dan's [[
> https://github.com/llvm/llvm-project/commit/1384ee936e46816f348bc3756704aeaff92e86fe
> | commit ]].
I did delete the overridden method below in
clang/lib/Driver/ToolChains/WebAssembly.cpp. This is restoring the base class
implementation to the way it was before your CL added the ArgList argument,
since we no longer need it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58742/new/
https://reviews.llvm.org/D58742
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
