[WebAssembly] Don't pass -ffunction-section/-fdata-sections

[sbc100]

Summary

llvm currently forces both of these to true to passing them
is redundant.

[sbc100]

• .

[dschuff]

I guess the reason for removing this is that we don't really have an analogue of different text sections in the wasm object format, but that dead function stripping can be done by any linker smart enough to link wasm at all. But this isn't necessarily true of data sections though, is it?

Also I don't understand why removing the hardcoding on the LLVM side requires removing the explicit passing of the options here?

[sbc100]

Making the change here first means that I can test the new default and land it in llvm. If clang is continues to set its own default then anyone using clang (i.e. a lot of the tests) won't see the effect of the change.

[dschuff]

Shouldn't the LLVM-side tests be using the LLVM utils and not clang? Or did you mean lld tests, or waterfall tests or what?

Once we have the support, I think we do want the default behavior to be to dead strip functions and data, and I agree that it shouldn't be hardcoded in LLVM. So this would be the natural way to do that, no?

[sbc100]

Yes, I was refering to the waterfall tests.

I don't see why webassembly should default to -fdata-sections/-ffunction-sections any more than any other platform should.

But I can hold off on this change if you like? I guess it isn't essential to the work i'm doing on the llvm side..

[dschuff]

I think the reasoning was really just that code size reduction is even more important on wasm than other platforms, and because it's a new ABI there's no one depending on the default behavior that would break.

And, I didn't mean to push back really hard or block you or anything, I just genuinely didn't understand what you meant in the commit description.

[sbc100]

• update tests

[sbc100]

After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning.

This means we don't need the driver to pass these options, which makes our commands lines shorter, and it also means that people who use the tools directly get the sensible defaults too.

[sunfish]

In D37831#871238, @dschuff wrote:

I think the reasoning was really just that code size reduction is even more important on wasm than other platforms, and because it's a new ABI there's no one depending on the default behavior that would break.

I agree. Also, on some platforms, -ffunction-sections and -fdata-sections can result in less efficient code, however that's not the case on wasm.

[dschuff]

In D37831#992822, @sbc100 wrote:

After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning.

Well sure, when you put it that way, nobody can argue against sensible defaults, can they :D

lib/Driver/ToolChains/CommonArgs.cpp

unsigned tools::getLTOParallelism(const ArgList &Args, const Driver &D) {
   Arg *LtoJobsArg = Args.getLastArg(options::OPT_flto_jobs_EQ);
   if (LtoJobsArg &&
       StringRef(LtoJobsArg->getValue()).getAsInteger(10, Parallelism))
     D.Diag(diag::err_drv_invalid_int_value) << LtoJobsArg->getAsString(Args)
                                             << LtoJobsArg->getValue();
   return Parallelism;
 }
 
-// CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
-// default.
+// CloudABI uses -ffunction-sections and -fdata-sections by default.
 bool tools::isUseSeparateSections(const llvm::Triple &Triple) {
-  return Triple.getOS() == llvm::Triple::CloudABI ||
-         Triple.getArch() == llvm::Triple::wasm32 ||
-         Triple.getArch() == llvm::Triple::wasm64;
+  return Triple.getOS() == llvm::Triple::CloudABI;
 }
 
 void tools::AddGoldPlugin(const ToolChain &ToolChain, const ArgList &Args,
                           ArgStringList &CmdArgs, bool IsThinLTO,
                           const Driver &D) {
   // Tell the linker to load the plugin. This has to come before AddLinkerInputs
   // as gold requires -plugin to come before any -plugin-opt that -Wl might
   // forward.

test/Driver/wasm-toolchain.c

 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
-// enabling -ffunction-sections, -fdata-sections, and -fvisibility=hidden by
-// default.
+// enabling -fvisibility=hidden by default.
 
 // RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 | FileCheck -check-prefix=CC1 %s
-// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}} "-ffunction-sections" "-fdata-sections"
-
-// Ditto, but ensure that a user -fno-function-sections disables the
-// default -ffunction-sections.
-
-// RUN: %clang %s -### -target wasm32-unknown-unknown -fno-function-sections 2>&1 | FileCheck -check-prefix=NO_FUNCTION_SECTIONS %s
-// NO_FUNCTION_SECTIONS-NOT: function-sections
-
-// Ditto, but ensure that a user -fno-data-sections disables the
-// default -fdata-sections.
-
-// RUN: %clang %s -### -target wasm32-unknown-unknown -fno-data-sections 2>&1 | FileCheck -check-prefix=NO_DATA_SECTIONS %s
-// NO_DATA_SECTIONS-NOT: data-sections
+// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
 // RUN: %clang %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C link command-line.

test/Driver/wasm-toolchain.cpp

 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
-// enabling -ffunction-sections, -fdata-sections, and -fvisibility=hidden by
-// default.
+// enabling -fvisibility=hidden by default.
 
 // RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 | FileCheck -check-prefix=CC1 %s
-// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}} "-ffunction-sections" "-fdata-sections"
-
-// Ditto, but ensure that a user -fno-function-sections disables the
-// default -ffunction-sections.
-
-// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fno-function-sections 2>&1 | FileCheck -check-prefix=NO_FUNCTION_SECTIONS %s
-// NO_FUNCTION_SECTIONS-NOT: function-sections
-
-// Ditto, but ensure that a user -fno-data-sections disables the
-// default -fdata-sections.
-
-// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fno-data-sections 2>&1 | FileCheck -check-prefix=NO_DATA_SECTIONS %s
-// NO_DATA_SECTIONS-NOT: data-sections
+// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
 // RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C++ link command-line.