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

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

2023-07-11 Thread Qiongsi Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0b66b3417c02: [clang][AIX] Fix Overly Strict LTO Option 
Checking against `data-sections` when… (authored by qiongsiwu1).

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/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/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -725,13 +725,16 @@
 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 @@
   << 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(


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

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

2023-07-11 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 updated this revision to Diff 539105.
qiongsiwu1 added a comment.

Fixing inconsistent braces. Thanks for the feedback @hubert.reinterpretcast !


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/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/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -727,13 +727,16 @@
 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)) {
@@ -746,8 +749,10 @@
   << 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(


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

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

2023-07-03 Thread Hubert Tong via Phabricator via cfe-commits

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

LGTM with minor comment.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:732-735
UseSeparateSections))
 CmdArgs.push_back(
 Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=1"));
+  else if (Args.hasArg(options::OPT_fno_data_sections)) {

Coding standards suggest that braces be used consistently for each part of an 
if/else chain.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152021/new/

https://reviews.llvm.org/D152021

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

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

2023-06-16 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 updated this revision to Diff 532099.
qiongsiwu1 added a comment.

Revert to the DataSections logic. It is better not to change that in this patch.


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/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/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -727,13 +727,16 @@
 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))
 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)) {
@@ -746,8 +749,10 @@
   << 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(


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

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information