[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin marked an inline comment as done. aheejin added inline comments. Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66 +if (Args.hasFlag(clang::driver::options::OPT_pthread, + clang::driver::options::OPT_no_pthread), +false)

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 185895. aheejin added a comment. Sorry nevermind my previous code. There was not hacky and much cleaner way to do everything in the driver layer. (Before I tried to do everything in the cc1 compilation layer :( ) Anyway, moved all logic to the driver layer

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Dan Gohman via Phabricator via cfe-commits
sunfish added a comment. That sounds reasonable to me too. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57874/new/ https://reviews.llvm.org/D57874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. Yes it makes sense to me to set `-mthread-model=posix` when `-pthread` is passed on the commandline. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57874/new/ https://reviews.llvm.org/D57874

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment. Oh I guess another option would just be to pin all 3 flags together here, but since `-pthread` sets a preprocessor define and may also affect linker behavior, I think it's fine to allow atomic codegen without setting `-pthread`. Repository: rC Clang CHANGES SINCE

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment. So this CL has the effect that setting `-pthreads` will also set `-matomics`. Currently as you mentioned we have the problem that we can't make our current logic of "do we lower away the atomics" be controlled by the target features because it's done at pass config time

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-06 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 185700. 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: lib/Frontend/CompilerInvocation.cpp test/Driver/wasm32-unknown-unknown.cpp

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-06 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin created this revision. aheejin added a reviewer: dschuff. Herald added subscribers: cfe-commits, jfb, sunfish, jgravelle-google, sbc100. Herald added a project: clang. In wasm, we always use '-matomics' when we use '-pthread'. This will make users type one less option and options will be