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

Moving the `-shared` check to the AIX linker and modifying the tests so we test 
pure linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/PowerPC/aix-roptr.c
  clang/test/Driver/ppc-roptr.c

Index: clang/test/Driver/ppc-roptr.c
===================================================================
--- /dev/null
+++ clang/test/Driver/ppc-roptr.c
@@ -0,0 +1,46 @@
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mxcoff-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -c -mxcoff-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mxcoff-roptr -mno-xcoff-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=NO_ROPTR
+
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -S -mxcoff-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=ROPTR
+// 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: touch %t.o
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %t.o 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=LINK
+
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mxcoff-roptr \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-xcoff-roptr \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: touch %t.o
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -shared \
+// RUN:     %t.o 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mxcoff-roptr -flto \
+// RUN:     %t.o 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -flto -fno-data-sections \
+// RUN:     %t.o 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mno-xcoff-roptr -flto -fno-data-sections \
+// RUN:     %t.o 2>&1 | FileCheck %s --check-prefix=NO_DATA_SECTION_ERR
+// 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
Index: clang/test/CodeGen/PowerPC/aix-roptr.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/PowerPC/aix-roptr.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mxcoff-roptr -fdata-sections \
+// RUN:     -S <%s | FileCheck %s --check-prefix=CHECK32
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mxcoff-roptr -fdata-sections \
+// RUN:     -S <%s | FileCheck %s --check-prefix=CHECK64
+// RUN: not %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mxcoff-roptr \
+// RUN:     -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mxcoff-roptr \
+// RUN:     -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -mxcoff-roptr \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+
+char c1 = 10;
+char c2 = 20;
+char* const c1_ptr = &c1;
+// CHECK32:         .csect c1_ptr[RO],2
+// CHECK32-NEXT:	.globl	c1_ptr[RO]
+// CHECK32-NEXT:	.align	2
+// CHECK32-NEXT:	.vbyte	4, c1[RW]
+
+// CHECK64:         .csect c1_ptr[RO],3
+// CHECK64-NEXT:	.globl	c1_ptr[RO]
+// CHECK64-NEXT:	.align	3
+// CHECK64-NEXT:	.vbyte	8, c1[RW]
+
+// DATA_SECTION_ERR: error: -mxcoff-roptr is supported only with -fdata-sections
+// TARGET_ROPTR_ERR: error: unsupported option '-mxcoff-roptr' for target 'powerpc64le-unknown-linux-gnu'
+
+int main() {
+    *(char**)&c1_ptr = &c2;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1560,6 +1560,9 @@
   if (Opts.EnableAIXExtendedAltivecABI)
     GenerateArg(Args, OPT_mabi_EQ_vec_extabi, SA);
 
+  if (Opts.XCOFFReadOnlyPointers)
+    GenerateArg(Args, OPT_mxcoff_roptr, SA);
+
   if (!Opts.OptRecordPasses.empty())
     GenerateArg(Args, OPT_opt_record_passes, Opts.OptRecordPasses, SA);
 
@@ -1949,6 +1952,25 @@
     Opts.EnableAIXExtendedAltivecABI = O.matches(OPT_mabi_EQ_vec_extabi);
   }
 
+  if (Arg *A = Args.getLastArg(OPT_mxcoff_roptr)) {
+    if (!T.isOSAIX())
+      Diags.Report(diag::err_drv_unsupported_opt_for_target)
+          << A->getSpelling() << T.str();
+
+    // Since the storage mapping class is specified per csect,
+    // without using data sections, it is less effective to use read-only
+    // pointers. Using read-only pointers may cause other RO variables in the
+    // same csect to become RW when the linker acts upon `-bforceimprw`;
+    // therefore, we require that separate data sections
+    // are used when `-mxcoff-roptr` is in effect. We respect the setting of
+    // data-sections since we have not found reasons to do otherwise that
+    // overcome the user surprise of not respecting the setting.
+    if (!Args.hasFlag(OPT_fdata_sections, OPT_fno_data_sections, false))
+      Diags.Report(diag::err_roptr_requires_data_sections);
+
+    Opts.XCOFFReadOnlyPointers = true;
+  }
+
   if (Arg *A = Args.getLastArg(OPT_mabi_EQ_quadword_atomics)) {
     if (!T.isOSAIX() || T.isPPC32())
       Diags.Report(diag::err_drv_unsupported_opt_for_target)
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -704,6 +704,26 @@
     CmdArgs.push_back(
         Args.MakeArgString(Twine(PluginOptPrefix) + "-data-sections=0"));
 
+  if (Args.hasArg(options::OPT_mxcoff_roptr) ||
+      Args.hasArg(options::OPT_mno_xcoff_roptr)) {
+    bool HasRoptr = Args.hasFlag(options::OPT_mxcoff_roptr,
+                                 options::OPT_mno_xcoff_roptr, false);
+    StringRef OptStr = HasRoptr ? "-mxcoff-roptr" : "-mno-xcoff-roptr";
+
+    if (!IsOSAIX)
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << OptStr << ToolChain.getTriple().str();
+
+    if (HasRoptr) {
+      // On AIX, data_sections is on by default. We only need to check
+      // if data_sections is explicitly turned off.
+      if (Args.hasArg(options::OPT_fno_data_sections))
+        D.Diag(diag::err_roptr_requires_data_sections);
+      CmdArgs.push_back(
+          Args.MakeArgString(Twine(PluginOptPrefix) + "-mxcoff-roptr"));
+    }
+  }
+
   // Pass an option to enable split machine functions.
   if (auto *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
                                 options::OPT_fno_split_machine_functions)) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5245,6 +5245,19 @@
           << A->getSpelling() << RawTriple.str();
   }
 
+  if (Args.hasArg(options::OPT_mxcoff_roptr) ||
+      Args.hasArg(options::OPT_mno_xcoff_roptr)) {
+    bool HasRoptr = Args.hasFlag(options::OPT_mxcoff_roptr,
+                                 options::OPT_mno_xcoff_roptr, false);
+    StringRef OptStr = HasRoptr ? "-mxcoff-roptr" : "-mno-xcoff-roptr";
+    if (!Triple.isOSAIX())
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << OptStr << RawTriple.str();
+
+    if (HasRoptr)
+      CmdArgs.push_back("-mxcoff-roptr");
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
     StringRef v = A->getValue();
     // FIXME: Validate the argument here so we don't produce meaningless errors
Index: clang/lib/Driver/ToolChains/AIX.cpp
===================================================================
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -122,6 +122,17 @@
     CmdArgs.push_back("-bnoentry");
   }
 
+  if (Args.hasFlag(options::OPT_mxcoff_roptr, options::OPT_mno_xcoff_roptr,
+                   false)) {
+    if (Args.hasArg(options::OPT_shared))
+      D.Diag(diag::err_roptr_cannot_build_shared);
+
+    // The `-mxcoff-roptr` option places constants in RO sections as much as
+    // possible. Then `-bforceimprw` changes such sections to RW if they contain
+    // imported symbols that needs to be resolved.
+    CmdArgs.push_back("-bforceimprw");
+  }
+
   // PGO instrumentation generates symbols belonging to special sections, and
   // the linker needs to place all symbols in a particular section together in
   // memory; the AIX linker does that under an option.
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -436,6 +436,7 @@
   Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
   Options.Hotpatch = CodeGenOpts.HotPatch;
   Options.JMCInstrument = CodeGenOpts.JMCInstrument;
+  Options.XCOFFReadOnlyPointers = CodeGenOpts.XCOFFReadOnlyPointers;
 
   switch (CodeGenOpts.getSwiftAsyncFramePointer()) {
   case CodeGenOptions::SwiftAsyncFramePointerKind::Auto:
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3894,6 +3894,9 @@
 def msvr4_struct_return : Flag<["-"], "msvr4-struct-return">,
   Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Return small structs in registers (PPC32 only)">;
+def mxcoff_roptr : Flag<["-"], "mxcoff-roptr">, Group<m_Group>, Flags<[CC1Option]>,
+  HelpText<"Place constant objects with relocatable address values in the RO data section and add -bforceimprw to the linker flags (AIX only)">;
+def mno_xcoff_roptr : Flag<["-"], "mno-xcoff-roptr">, Group<m_Group>;
 
 def mvx : Flag<["-"], "mvx">, Group<m_Group>;
 def mno_vx : Flag<["-"], "mno-vx">, Group<m_Group>;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -643,6 +643,8 @@
   "OBJECT_MODE setting %0 is not recognized and is not a valid setting">;
 
 def err_aix_unsupported_tls_model : Error<"TLS model '%0' is not yet supported on AIX">;
+def err_roptr_requires_data_sections: Error<"-mxcoff-roptr is supported only with -fdata-sections">;
+def err_roptr_cannot_build_shared: Error<"-mxcoff-roptr is not supported with -shared">;
 
 def err_invalid_cxx_abi : Error<"invalid C++ ABI name '%0'">;
 def err_unsupported_cxx_abi : Error<"C++ ABI '%0' is not supported on target triple '%1'">;
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -52,6 +52,7 @@
                                                ///< Produce unique section names with
                                                ///< basic block sections.
 CODEGENOPT(EnableAIXExtendedAltivecABI, 1, 0) ///< Set for -mabi=vec-extabi. Enables the extended Altivec ABI on AIX.
+CODEGENOPT(XCOFFReadOnlyPointers, 1, 0) ///< Set for -mxcoff-roptr.
 ENUM_CODEGENOPT(FramePointer, FramePointerKind, 2, FramePointerKind::None) /// frame-pointer: all,non-leaf,none
 
 CODEGENOPT(ClearASTBeforeBackend , 1, 0) ///< Free the AST before running backend code generation. Only works with -disable-free.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -343,6 +343,12 @@
   This option is an alternative to the `--build-id=0xHEXSTRING` GNU linker option
   which is currently not supported by the AIX linker.
 
+- Introduced the ``-mxcoff-roptr`` option to place constant objects with
+  relocatable address values in the read-only data section. This option should
+  be used with the ``-fdata-sections`` option, and is not supported with
+  ``-fno-data-sections``. When ``-mxcoff-roptr`` is in effect at link time,
+  read-only data sections with relocatable address values that resolve to
+  imported symbols are made writable.
 
 WebAssembly Support
 ^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to