[PATCH] D152021: [clang][AIX] Fix Overly Strick LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

[Qiongsi Wu via Phabricator via cfe-commits]

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(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152021.530903.patch
Type: text/x-patch
Size: 4635 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230613/f22cec55/attachment-0001.bin>