qiongsiwu1 updated this revision to Diff 530903.
qiongsiwu1 added a comment.
Revising the logic to pass `-data-sections=` to `libLTO`. We now pass
`-data-sections=1` to `libLTO` by default unless it is explicitly turned off.
Old behaviour:
1. Pass `-data-sections=1` if the target requires uses of separate sections, or
the user explicitly sets `-fdata-sections`.
2. Pass `-data-sections=0` if the user explicitly specifies
`-fno-data-sections`.
3. Pass nothing otherwise.
New behaviour:
1. Pass `-data-section=1` if nothing is specified, or if the target requires
uses of separate sections, or the user explicitly sets `-fdata-sections`.
2. Pass `-data-sections=0` if the user explicitly specifies
`-fno-data-sections`.
I think the new behaviour is safe because `libLTO` defaults to use data
sections when nothing is specified
https://github.com/llvm/llvm-project/blob/829ed96b779c10cb023d7e3dbcfad22413979ec4/llvm/lib/LTO/LTOCodeGenerator.cpp#L426.
@hubert.reinterpretcast @MaskRay could you let me know what you think? Thanks
so much!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152021/new/
https://reviews.llvm.org/D152021
Files:
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/function-sections.c
clang/test/Driver/hip-options.hip
clang/test/Driver/ppc-roptr.c
Index: clang/test/Driver/ppc-roptr.c
===================================================================
--- clang/test/Driver/ppc-roptr.c
+++ clang/test/Driver/ppc-roptr.c
@@ -12,7 +12,7 @@
// RUN: %clang -### --target=powerpc-ibm-aix-xcoff %s 2>&1 | \
// RUN: FileCheck %s --check-prefix=NO_ROPTR
// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -flto %s
2>&1 | \
-// RUN: FileCheck %s --check-prefixes=ROPTR,LINK,LTO_ROPTR
+// RUN: FileCheck %s
--check-prefixes=NO_DATA_SECTION_ERR,ROPTR,LINK,LTO_ROPTR
// RUN: touch %t.o
// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %t.o 2>&1 |
\
// RUN: FileCheck %s --check-prefix=LINK
@@ -33,14 +33,14 @@
// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-xcoff-roptr
-flto \
// RUN: %t.o 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
-// ROPTR: "-mxcoff-roptr"
-// LINK: "-bforceimprw"
-// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
-// NO_ROPTR-NOT: "-mxcoff-roptr"
-// NO_ROPTR-NOT: "-bforceimprw"
-
// DATA_SECTION_ERR: error: -mxcoff-roptr is supported only with
-fdata-sections
// NO_DATA_SECTION_ERR-NOT: error: -mxcoff-roptr is supported only with
-fdata-sections
// TARGET_ROPTR_ERR: error: unsupported option '-mxcoff-roptr' for target
'powerpc64le-unknown-linux-gnu'
// TARGET_NOROPTR_ERR: error: unsupported option '-mno-xcoff-roptr' for target
'powerpc64le-unknown-linux-gnu'
// SHARED_ERR: error: -mxcoff-roptr is not supported with -shared
+
+// ROPTR: "-mxcoff-roptr"
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
+// NO_ROPTR-NOT: "-bforceimprw"
Index: clang/test/Driver/hip-options.hip
===================================================================
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -79,7 +79,7 @@
// HIPTHINLTO-NOT: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}}
"-flto-unit"
// HIPTHINLTO: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-flto=thin"
"-flto-unit" {{.*}} "-fwhole-program-vtables"
// HIPTHINLTO-NOT: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}}
"-flto-unit"
-// HIPTHINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto"
"-plugin-opt=-force-import-all"
+// HIPTHINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto" {{.*}}
"-plugin-opt=-force-import-all"
// Check that -flto=thin is handled correctly, particularly with
-fwhole-program-vtables.
//
Index: clang/test/Driver/function-sections.c
===================================================================
--- clang/test/Driver/function-sections.c
+++ clang/test/Driver/function-sections.c
@@ -7,7 +7,6 @@
// CHECK-US-NOT: -fno-unique-section-names
// CHECK-NOUS: -fno-unique-section-names
// CHECK-PLUGIN-DEFAULT-NOT: "-plugin-opt=-function-sections
-// CHECK-PLUGIN-DEFAULT-NOT: "-plugin-opt=-data-sections
// CHECK-PLUGIN-SECTIONS: "-plugin-opt=-function-sections=1"
// CHECK-PLUGIN-SECTIONS: "-plugin-opt=-data-sections=1"
// CHECK-PLUGIN-NO-SECTIONS: "-plugin-opt=-function-sections=0"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -727,13 +727,14 @@
CmdArgs.push_back(
Args.MakeArgString(Twine(PluginOptPrefix) + "-function-sections=0"));
- if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
- UseSeparateSections))
- CmdArgs.push_back(
- Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=1"));
- else if (Args.hasArg(options::OPT_fno_data_sections))
- CmdArgs.push_back(
- Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=0"));
+ bool UseDataSections =
+ Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections) ||
+ !Args.hasArg(options::OPT_fno_data_sections);
+ StringRef DataSectionsOpt =
+ UseDataSections ? "-data-sections=1" : "-data-sections=0";
+ CmdArgs.push_back(
+ Args.MakeArgString(Twine(PluginOptPrefix) + DataSectionsOpt));
if (Args.hasArg(options::OPT_mxcoff_roptr) ||
Args.hasArg(options::OPT_mno_xcoff_roptr)) {
@@ -746,8 +747,7 @@
<< OptStr << ToolChain.getTriple().str();
if (HasRoptr) {
- if (!Args.hasFlag(options::OPT_fdata_sections,
- options::OPT_fno_data_sections, UseSeparateSections))
+ if (!UseDataSections)
D.Diag(diag::err_roptr_requires_data_sections);
CmdArgs.push_back(
Index: clang/test/Driver/ppc-roptr.c
===================================================================
--- clang/test/Driver/ppc-roptr.c
+++ clang/test/Driver/ppc-roptr.c
@@ -12,7 +12,7 @@
// RUN: %clang -### --target=powerpc-ibm-aix-xcoff %s 2>&1 | \
// RUN: FileCheck %s --check-prefix=NO_ROPTR
// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -flto %s 2>&1 | \
-// RUN: FileCheck %s --check-prefixes=ROPTR,LINK,LTO_ROPTR
+// RUN: FileCheck %s --check-prefixes=NO_DATA_SECTION_ERR,ROPTR,LINK,LTO_ROPTR
// RUN: touch %t.o
// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %t.o 2>&1 | \
// RUN: FileCheck %s --check-prefix=LINK
@@ -33,14 +33,14 @@
// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-xcoff-roptr -flto \
// RUN: %t.o 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
-// ROPTR: "-mxcoff-roptr"
-// LINK: "-bforceimprw"
-// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
-// NO_ROPTR-NOT: "-mxcoff-roptr"
-// NO_ROPTR-NOT: "-bforceimprw"
-
// DATA_SECTION_ERR: error: -mxcoff-roptr is supported only with -fdata-sections
// NO_DATA_SECTION_ERR-NOT: error: -mxcoff-roptr is supported only with -fdata-sections
// TARGET_ROPTR_ERR: error: unsupported option '-mxcoff-roptr' for target 'powerpc64le-unknown-linux-gnu'
// TARGET_NOROPTR_ERR: error: unsupported option '-mno-xcoff-roptr' for target 'powerpc64le-unknown-linux-gnu'
// SHARED_ERR: error: -mxcoff-roptr is not supported with -shared
+
+// ROPTR: "-mxcoff-roptr"
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
+// NO_ROPTR-NOT: "-bforceimprw"
Index: clang/test/Driver/hip-options.hip
===================================================================
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -79,7 +79,7 @@
// HIPTHINLTO-NOT: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto-unit"
// HIPTHINLTO: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-flto=thin" "-flto-unit" {{.*}} "-fwhole-program-vtables"
// HIPTHINLTO-NOT: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto-unit"
-// HIPTHINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto" "-plugin-opt=-force-import-all"
+// HIPTHINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto" {{.*}} "-plugin-opt=-force-import-all"
// Check that -flto=thin is handled correctly, particularly with -fwhole-program-vtables.
//
Index: clang/test/Driver/function-sections.c
===================================================================
--- clang/test/Driver/function-sections.c
+++ clang/test/Driver/function-sections.c
@@ -7,7 +7,6 @@
// CHECK-US-NOT: -fno-unique-section-names
// CHECK-NOUS: -fno-unique-section-names
// CHECK-PLUGIN-DEFAULT-NOT: "-plugin-opt=-function-sections
-// CHECK-PLUGIN-DEFAULT-NOT: "-plugin-opt=-data-sections
// CHECK-PLUGIN-SECTIONS: "-plugin-opt=-function-sections=1"
// CHECK-PLUGIN-SECTIONS: "-plugin-opt=-data-sections=1"
// CHECK-PLUGIN-NO-SECTIONS: "-plugin-opt=-function-sections=0"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -727,13 +727,14 @@
CmdArgs.push_back(
Args.MakeArgString(Twine(PluginOptPrefix) + "-function-sections=0"));
- if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
- UseSeparateSections))
- CmdArgs.push_back(
- Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=1"));
- else if (Args.hasArg(options::OPT_fno_data_sections))
- CmdArgs.push_back(
- Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=0"));
+ bool UseDataSections =
+ Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
+ UseSeparateSections) ||
+ !Args.hasArg(options::OPT_fno_data_sections);
+ StringRef DataSectionsOpt =
+ UseDataSections ? "-data-sections=1" : "-data-sections=0";
+ CmdArgs.push_back(
+ Args.MakeArgString(Twine(PluginOptPrefix) + DataSectionsOpt));
if (Args.hasArg(options::OPT_mxcoff_roptr) ||
Args.hasArg(options::OPT_mno_xcoff_roptr)) {
@@ -746,8 +747,7 @@
<< OptStr << ToolChain.getTriple().str();
if (HasRoptr) {
- if (!Args.hasFlag(options::OPT_fdata_sections,
- options::OPT_fno_data_sections, UseSeparateSections))
+ if (!UseDataSections)
D.Diag(diag::err_roptr_requires_data_sections);
CmdArgs.push_back(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
