[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:647
+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 suppored with -shared">;
 
----------------
Typo.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5258-5259
+    if (HasRoptr) {
+      if (Args.hasArg(options::OPT_shared))
+        D.Diag(diag::err_roptr_cannot_build_shared);
+
----------------
I think this belongs in the code for AIX linker jobs.


================
Comment at: clang/test/Driver/ppc-roptr.c:37
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
----------------
This needs the backend option also renamed. Has a commit for that landed yet?


================
Comment at: clang/test/Driver/ppc-roptr.c:15
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=LTO_ROPTR
+// RUN: touch %t.o
----------------
This is compile+LTO link. We can also add `ROPTR,LINK` prefixes here, right?


================
Comment at: clang/test/Driver/ppc-roptr.c:24
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -shared \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
----------------
Please make this case pure-link. The diagnostic is most relevant on the link step because that is when `-shared` takes effect.


================
Comment at: clang/test/Driver/ppc-roptr.c:26
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mroptr -flto \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
----------------
Please make this case pure-link; otherwise, we will pass the test simply because the diagnostic is also implemented for compiling-from-source.


================
Comment at: clang/test/Driver/ppc-roptr.c:28
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto -fno-data-sections \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
----------------
Same comment.


================
Comment at: clang/test/Driver/ppc-roptr.c:30
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-roptr -flto \
+// RUN:     %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
----------------
Same comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190