[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
fhahn added a comment. In https://reviews.llvm.org/D24644#792262, @mehdi_amini wrote: > In https://reviews.llvm.org/D24644#792168, @fhahn wrote: > > > I'd like to fix PR22999 and was wondering if you think adding a > > function-section attribute to the IR would be a viable solution? > > > > When doing LTO, we could add the same function-section to each function in > > a module in the IRLinker. @mehdi_amini did you think something like that > > when suggesting using attributes? > > > Not sure how this would work? How do you codegen half of the functions with > function-section but not all? How is the backend supposed to behave if it > starts with a function that isn't decorated with this attribute, then move to > one that is, and finally proceed with one that isn't? hm yes I guess we would have to group the functions by their function-section attribute, which isn't such a good idea probably. @echristo you mentioned that the compiler maybe could determine if we should use function-sections during codegen, but it does not seem possible to determine the size of the code section before emitting all functions, at which point it is already too late. Maybe I am missing something? Repository: rL LLVM https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
mehdi_amini added a comment. In https://reviews.llvm.org/D24644#792168, @fhahn wrote: > I'd like to fix PR22999 and was wondering if you think adding a > function-section attribute to the IR would be a viable solution? > > When doing LTO, we could add the same function-section to each function in a > module in the IRLinker. @mehdi_amini did you think something like that when > suggesting using attributes? Not sure how this would work? How do you codegen half of the functions with function-section but not all? How is the backend supposed to behave if it starts with a function that isn't decorated with this attribute, then move to one that is, and finally proceed with one that isn't? Repository: rL LLVM https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
fhahn added a comment. I'd like to fix PR22999 and was wondering if you think adding a function-section attribute to the IR would be a viable solution? When doing LTO, we could add the same function-section to each function in a module in the IRLinker. @mehdi_amini did you think something like that when suggesting using attributes? ps: Maybe it would be better to discuss this at the bug report, but this review seemed the easiest way to address the right people. Repository: rL LLVM https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284140: Pass -ffunction-sections/-fdata-sections along to
gold-plugin (authored by tejohnson).
Changed prior to commit:
https://reviews.llvm.org/D24644?vs=71591=74555#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24644
Files:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/gold-lto-sections.c
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -2002,6 +2002,14 @@
return Parallelism;
}
+// CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
+// default.
+static bool isUseSeparateSections(const llvm::Triple ) {
+ return Triple.getOS() == llvm::Triple::CloudABI ||
+ Triple.getArch() == llvm::Triple::wasm32 ||
+ Triple.getArch() == llvm::Triple::wasm64;
+}
+
static void AddGoldPlugin(const ToolChain , const ArgList ,
ArgStringList , bool IsThinLTO,
const Driver ) {
@@ -2051,6 +2059,19 @@
else
CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
}
+
+ bool UseSeparateSections =
+ isUseSeparateSections(ToolChain.getEffectiveTriple());
+
+ if (Args.hasFlag(options::OPT_ffunction_sections,
+ options::OPT_fno_function_sections, UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-function-sections");
+ }
+
+ if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-data-sections");
+ }
}
/// This is a helper function for validating the optional refinement step
@@ -4763,11 +4784,7 @@
CmdArgs.push_back("-generate-type-units");
}
- // CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
- // default.
- bool UseSeparateSections = Triple.getOS() == llvm::Triple::CloudABI ||
- Triple.getArch() == llvm::Triple::wasm32 ||
- Triple.getArch() == llvm::Triple::wasm64;
+ bool UseSeparateSections = isUseSeparateSections(Triple);
if (Args.hasFlag(options::OPT_ffunction_sections,
options::OPT_fno_function_sections, UseSeparateSections)) {
Index: cfe/trunk/test/Driver/gold-lto-sections.c
===
--- cfe/trunk/test/Driver/gold-lto-sections.c
+++ cfe/trunk/test/Driver/gold-lto-sections.c
@@ -0,0 +1,8 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN: -Wl,-plugin-opt=foo -O3 \
+// RUN: -ffunction-sections -fdata-sections \
+// RUN: | FileCheck %s
+// CHECK: "-plugin-opt=-function-sections"
+// CHECK: "-plugin-opt=-data-sections"
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -2002,6 +2002,14 @@
return Parallelism;
}
+// CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
+// default.
+static bool isUseSeparateSections(const llvm::Triple ) {
+ return Triple.getOS() == llvm::Triple::CloudABI ||
+ Triple.getArch() == llvm::Triple::wasm32 ||
+ Triple.getArch() == llvm::Triple::wasm64;
+}
+
static void AddGoldPlugin(const ToolChain , const ArgList ,
ArgStringList , bool IsThinLTO,
const Driver ) {
@@ -2051,6 +2059,19 @@
else
CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
}
+
+ bool UseSeparateSections =
+ isUseSeparateSections(ToolChain.getEffectiveTriple());
+
+ if (Args.hasFlag(options::OPT_ffunction_sections,
+ options::OPT_fno_function_sections, UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-function-sections");
+ }
+
+ if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-data-sections");
+ }
}
/// This is a helper function for validating the optional refinement step
@@ -4763,11 +4784,7 @@
CmdArgs.push_back("-generate-type-units");
}
- // CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
- // default.
- bool UseSeparateSections = Triple.getOS() == llvm::Triple::CloudABI ||
- Triple.getArch() == llvm::Triple::wasm32 ||
- Triple.getArch() == llvm::Triple::wasm64;
+ bool UseSeparateSections = isUseSeparateSections(Triple);
if (Args.hasFlag(options::OPT_ffunction_sections,
options::OPT_fno_function_sections, UseSeparateSections)) {
Index: cfe/trunk/test/Driver/gold-lto-sections.c
===
--- cfe/trunk/test/Driver/gold-lto-sections.c
+++
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
tejohnson added a comment. Ping. It seems like using attributes is not feasible at this time due to the lack of data attributes. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
echristo added a comment. In https://reviews.llvm.org/D24644#562286, @mehdi_amini wrote: > What about function attributes? Hey that's the trend :) > You could have a subset of the functions in their own sections but not all. > With LTO it means that you can disable this for a single input file. True, but we'd need data attributes too for -fdata-sections. That's the main reason I haven't migrated the options out of TargetOptions and into the IR here. Rough sketch on module merge time: We'd probably want to error on functions/data that had separate section set in one module but not in another - there are a few ways to make that not error at link time, but at that point you're really relying on weird linker side effects and it's probably not what you intended anyhow. -eric https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
mehdi_amini added a comment. What about function attributes? Hey that's the trend :) You could have a subset of the functions in their own sections but not all. With LTO it means that you can disable this for a single input file. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
echristo added a comment. There are two things pushing and pulling here: a) You want to be able to pass compiler code generation options at the time we're actually doing the code generation, b) "Traditionally" we don't pass CFLAGS to the linker. I think I'd like to see us passing more options down at code generation time and handling it explicitly. In particular for this we don't have the knowledge at link time of what was intended. Even if we only turn it on when we see -gc-sections we won't know if the programmer wants function and data sections or just the former. Or maybe they want function sections for some reason other than gc-sections. In short, I'm more on the a) side here in what I want :) To go to PR22999: I think it might be reasonable for the linker during code generation to turn on function/data-sections where it isn't reasonable for us to do so in the compiler. I can come up with weird cases to break it, but those are largely module level inline assembly. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
pcc added a comment. > Are you suggesting having them enabled unconditionally in both gold-plugin > and gold? That will require changes to both llvm and binutils, and the latter > will have effects for other compilers. I mean in the gold plugin only. There would not need to be any changes to gold because it does not take these parameters directly. > What about -Wl,-gc-sections, isn't that also needed to make effective use of > these options? Not necessarily, PR22999 would happen with or without gc-sections. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
tejohnson added a comment. In https://reviews.llvm.org/D24644#557774, @pcc wrote: > Perhaps it would be better to enable -ffunction-sections/-fdata-sections > unconditionally at the linker level if the linker supports it? See also > PR22999. My understanding is that these are not typically the default because it makes the resulting object files larger and the link slower. OTOH, for ThinLTO we definitely benefit more from these optimizations (and from PR22999 looks like there is a compelling reason to want this for LTO as well). However, when you say enable them "unconditionally at the linker level" what do you mean - these need to go LLVM via plugin options. Are you suggesting having them enabled unconditionally in both gold-plugin and gold? That will require changes to both llvm and binutils, and the latter will have effects for other compilers. What about -Wl,-gc-sections, isn't that also needed to make effective use of these options? https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
pcc added a comment. Perhaps it would be better to enable -ffunction-sections/-fdata-sections unconditionally at the linker level if the linker supports it? See also PR22999. https://reviews.llvm.org/D24644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24644: Pass -ffunction-sections/-fdata-sections along to gold-plugin
tejohnson created this revision.
tejohnson added reviewers: mehdi_amini, pcc.
tejohnson added a subscriber: cfe-commits.
Herald added subscribers: mehdi_amini, dschuff, jfb.
These options need to be passed to the plugin in order to have
an effect on LTO/ThinLTO compiles.
https://reviews.llvm.org/D24644
Files:
lib/Driver/Tools.cpp
test/Driver/gold-lto-sections.c
Index: test/Driver/gold-lto-sections.c
===
--- /dev/null
+++ test/Driver/gold-lto-sections.c
@@ -0,0 +1,8 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN: -Wl,-plugin-opt=foo -O3 \
+// RUN: -ffunction-sections -fdata-sections \
+// RUN: | FileCheck %s
+// CHECK: "-plugin-opt=-function-sections"
+// CHECK: "-plugin-opt=-data-sections"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2002,6 +2002,14 @@
}
}
+// CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
+// default.
+static bool isUseSeparateSections(const llvm::Triple ) {
+ return Triple.getOS() == llvm::Triple::CloudABI ||
+ Triple.getArch() == llvm::Triple::wasm32 ||
+ Triple.getArch() == llvm::Triple::wasm64;
+}
+
static void AddGoldPlugin(const ToolChain , const ArgList ,
ArgStringList , bool IsThinLTO) {
// Tell the linker to load the plugin. This has to come before
AddLinkerInputs
@@ -2046,6 +2054,19 @@
else
CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
}
+
+ bool UseSeparateSections =
+ isUseSeparateSections(ToolChain.getEffectiveTriple());
+
+ if (Args.hasFlag(options::OPT_ffunction_sections,
+ options::OPT_fno_function_sections, UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-function-sections");
+ }
+
+ if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-data-sections");
+ }
}
/// This is a helper function for validating the optional refinement step
@@ -4743,11 +4764,7 @@
CmdArgs.push_back("-generate-type-units");
}
- // CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
- // default.
- bool UseSeparateSections = Triple.getOS() == llvm::Triple::CloudABI ||
- Triple.getArch() == llvm::Triple::wasm32 ||
- Triple.getArch() == llvm::Triple::wasm64;
+ bool UseSeparateSections = isUseSeparateSections(Triple);
if (Args.hasFlag(options::OPT_ffunction_sections,
options::OPT_fno_function_sections, UseSeparateSections)) {
Index: test/Driver/gold-lto-sections.c
===
--- /dev/null
+++ test/Driver/gold-lto-sections.c
@@ -0,0 +1,8 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN: -Wl,-plugin-opt=foo -O3 \
+// RUN: -ffunction-sections -fdata-sections \
+// RUN: | FileCheck %s
+// CHECK: "-plugin-opt=-function-sections"
+// CHECK: "-plugin-opt=-data-sections"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2002,6 +2002,14 @@
}
}
+// CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
+// default.
+static bool isUseSeparateSections(const llvm::Triple ) {
+ return Triple.getOS() == llvm::Triple::CloudABI ||
+ Triple.getArch() == llvm::Triple::wasm32 ||
+ Triple.getArch() == llvm::Triple::wasm64;
+}
+
static void AddGoldPlugin(const ToolChain , const ArgList ,
ArgStringList , bool IsThinLTO) {
// Tell the linker to load the plugin. This has to come before AddLinkerInputs
@@ -2046,6 +2054,19 @@
else
CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
}
+
+ bool UseSeparateSections =
+ isUseSeparateSections(ToolChain.getEffectiveTriple());
+
+ if (Args.hasFlag(options::OPT_ffunction_sections,
+ options::OPT_fno_function_sections, UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-function-sections");
+ }
+
+ if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections)) {
+CmdArgs.push_back("-plugin-opt=-data-sections");
+ }
}
/// This is a helper function for validating the optional refinement step
@@ -4743,11 +4764,7 @@
CmdArgs.push_back("-generate-type-units");
}
- // CloudABI and WebAssembly use -ffunction-sections and -fdata-sections by
- // default.
- bool UseSeparateSections = Triple.getOS() == llvm::Triple::CloudABI ||
- Triple.getArch() == llvm::Triple::wasm32 ||
- Triple.getArch() == llvm::Triple::wasm64;
+ bool
