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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323897: [WebAssembly] Dont pass 
-ffunction-section/-fdata-sections (authored by sbc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D37831?vs=132098=132213#toc

Repository:
  rC Clang

https://reviews.llvm.org/D37831

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -363,12 +363,9 @@
   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 ) {
-  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 , const ArgList ,
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -1,21 +1,8 @@
 // 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.
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -1,21 +1,8 @@
 // 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.


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -363,12 +363,9 @@
   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 ) {
-  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 , const ArgList ,
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -1,21 +1,8 @@
 // A basic clang -cc1 command-line. 

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

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In https://reviews.llvm.org/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


Repository:
  rC Clang

https://reviews.llvm.org/D37831



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/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.


Repository:
  rC Clang

https://reviews.llvm.org/D37831



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D37831



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37831: [WebAssembly] Don't pass -ffunction-section/-fdata-sections by default

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 132098.
sbc100 added a comment.
Herald added subscribers: cfe-commits, sunfish.

- update tests


Repository:
  rC Clang

https://reviews.llvm.org/D37831

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp


Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -1,21 +1,8 @@
 // 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.
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -1,21 +1,8 @@
 // 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.
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -363,12 +363,9 @@
   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 ) {
-  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 , const ArgList ,


Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -1,21 +1,8 @@
 // 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