[clang] 0b66b34 - [clang][AIX] Fix Overly Strict LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

[Qiongsi Wu via cfe-commits]

Author: Qiongsi Wu
Date: 2023-07-11T13:24:09-04:00
New Revision: 0b66b3417c026e145708d0e20bfd05a72e79f12a

URL: https://github.com/llvm/llvm-project/commit/0b66b3417c026e145708d0e20bfd05a72e79f12a
DIFF: https://github.com/llvm/llvm-project/commit/0b66b3417c026e145708d0e20bfd05a72e79f12a.diff

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

The LTO `-mxcoff-roptr` [[ https://github.com/llvm/llvm-project/blob/c6b2d25927817bdeca99653ee3e66720f33ce3ae/clang/lib/Driver/ToolChains/CommonArgs.cpp#L750 | check ]] against data sections is overly strict and it ignores the fact that [[ https://github.com/llvm/llvm-project/blob/c6b2d25927817bdeca99653ee3e66720f33ce3ae/llvm/lib/LTO/LTOCodeGenerator.cpp#L427 | data sections is on by default on AIX ]], causing valid LTO compilation to fail when `-fdata-sections` is not explicitly specified.

This patch revises the check so that an error is reported only if data sections is explicitly turned off for LTO.

Reviewed By: hubert.reinterpretcast

Differential Revision: https://reviews.llvm.org/D152021

Added: 
    

Modified: 
    clang/lib/Driver/ToolChains/CommonArgs.cpp
    clang/test/Driver/ppc-roptr.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index a58058fdcd6c3d..3dcbd5b8deb8d7 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -725,13 +725,16 @@ void tools::addLTOOptions(const ToolChain &ToolChain, const ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString(Twine(PluginOptPrefix) + "-function-sections=0"));
 
+  bool DataSectionsTurnedOff = false;
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
-                   UseSeparateSections))
+                   UseSeparateSections)) {
     CmdArgs.push_back(
         Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=1"));
-  else if (Args.hasArg(options::OPT_fno_data_sections))
+  } else if (Args.hasArg(options::OPT_fno_data_sections)) {
+    DataSectionsTurnedOff = true;
     CmdArgs.push_back(
         Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=0"));
+  }
 
   if (Args.hasArg(options::OPT_mxcoff_roptr) ||
       Args.hasArg(options::OPT_mno_xcoff_roptr)) {
@@ -744,8 +747,10 @@ void tools::addLTOOptions(const ToolChain &ToolChain, const ArgList &Args,
           << OptStr << ToolChain.getTriple().str();
 
     if (HasRoptr) {
-      if (!Args.hasFlag(options::OPT_fdata_sections,
-                        options::OPT_fno_data_sections, UseSeparateSections))
+      // The data sections option is on by default on AIX. We only need to error
+      // out when -fno-data-sections is specified explicitly to turn off data
+      // sections.
+      if (DataSectionsTurnedOff)
         D.Diag(diag::err_roptr_requires_data_sections);
 
       CmdArgs.push_back(

diff  --git a/clang/test/Driver/ppc-roptr.c b/clang/test/Driver/ppc-roptr.c
index 0ab740da9e04a5..d32bbf622ad889 100644
--- a/clang/test/Driver/ppc-roptr.c
+++ b/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"