[WebAssembly] Make driver -pthread imply linker --shared-memory

[tlively]

Summary

This eliminates a linker error the user might otherwise see about how
using the 'atomics' feature requires --shared-memory.

[phosek]

This broke our builders (see https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8918246038501974992/+/steps/clang/0/steps/test/0/stdout) because the test uses the default linker (lld in our case) instead of wasm-ld, it seems that the clang invocation on line 45 is missing the -fuse-ld=wasm-ld argument.

[sbc100]

Hi @phosek !

Fixed in rL356847.

This did make me look a little deeper into this issue though, and wonder if the CLANG_DEFAULT_LINKER is implemented in a useful way right now.

I believe the reason this came up for you guys in that you are somehow setting CLANG_DEFAULT_LINKER? Not on the command line but perhaps via the clang/cmake/caches/Fuchsia.cmake and clang/cmake/caches/Fuchsia-stage2.cmake? In regards to that I have two questions:

1. Do you need to set CLANG_DEFAULT_LINKER give that you already override getDefaultLinker in clang/lib/Driver/ToolChains/Fuchsia.h? I suppose it might be useful if you want to overide the linker for other targets too?

2. Does it makes sense for CLANG_DEFAULT_LINKER to able able to override a target-specific linker? In the case of WebAssembly it makes no sense to be able to configure an ELF or COFF linker as that default so it would be good for us if our getDefaultLinker still stands even in the presence of CLANG_DEFAULT_LINKER.

Assuming the answer to (1) is "yes we are doing this for some good reason", then what do you think about moving the logic for handling CLANG_DEFAULT_LINKER into the default implementation of getDefaultLinker in clang/include/clang/Driver/ToolChain.h. That way it effects the default, but targets can still override the default if they actually care? They way the linker resolution would look more like this:

1. the command line
2. the target-specific default
3. the toolchain-wide default (settable via CLANG_DEFAULT_LINKER)

cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp

if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
     CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt1.o")));
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     if (ToolChain.ShouldLinkCXXStdlib(Args))
       ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
 
-    if (Args.hasArg(options::OPT_pthread))
+    if (Args.hasArg(options::OPT_pthread)) {
       CmdArgs.push_back("-lpthread");
+      CmdArgs.push_back("--shared-memory");
+    }
 
     CmdArgs.push_back("-lc");
     AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args);
   }
 
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 

cfe/trunk/test/Driver/wasm-toolchain.c

 
 // A basic C compile command-line with known OS.
 
 // 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.
 
-// '-pthread' sets '-target-feature +atomics'
+// '-pthread' sets '-target-feature +atomics' and '--shared-memory'
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
 // PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+// PTHREAD: wasm-ld{{.*}}" "-lpthread" "--shared-memory"
 
 // '-pthread' not allowed with '-mno-atomics'
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mno-atomics 2>&1 | FileCheck -check-prefix=PTHREAD_NO_ATOMICS %s
 // PTHREAD_NO_ATOMICS: invalid argument '-pthread' not allowed with '-mno-atomics'