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

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/docs/ReleaseNotes.rst:231
+  address values in the read-only data section. This option is intended to
+  be used with the ``-fdata-sections`` option. When ``-mroptr`` is in effect,
+  read-only data sections with relocatable address values that resolve to
----------------
Suggestion:
is in effect<ins> at link time</ins>,


================
Comment at: clang/test/CodeGen/PowerPC/aix-roptr.c:9-10
+// RUN:     -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -mroptr \
+// RUN:     -S <%s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -mno-roptr \
----------------
hubert.reinterpretcast wrote:
> Note that a driver test is needed for this sort of case as well.
`-mroptr` with a wrong target is currently also a front-end diagnostic (which we should then be testing here).

Having such a front-end diagnostic makes sense to me because the `-cc1` option may get silently ignored otherwise: the back-end proper does not ensure that it is unset for other targets (instead `llc` has the checking).


================
Comment at: clang/test/Driver/ppc-roptr.c:6
+// RUN:     FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=NO_ROPTR
----------------
May also want to test that the default behaviour is `NO_ROPTR`.


================
Comment at: clang/test/Driver/ppc-roptr.c:1
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 | \
----------------
qiongsiwu1 wrote:
> hubert.reinterpretcast wrote:
> > Should test for `-c` and `-S` as well.
> > Should test for pure link (without compile) as well.
> > Should test for pure link (without compile) as well.
> 
> Is there a set of options that I can use to trigger pure linking? Or should I use two .o input files to trigger pure linking? 
Just one `.o` file will trigger pure linking. For example, some tests do a `touch %t.o`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190