[PATCH] D153907: [AIX] [TOC] Add -mtocdata/-mno-tocdata options on AIX

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm added inline comments.


================
Comment at: clang/lib/CodeGen/Targets/PPC.cpp:274
+  if (auto *GVar = dyn_cast<llvm::GlobalVariable>(GV)) {
+    auto GVId = GVar->getGlobalIdentifier();
+
----------------
hubert.reinterpretcast wrote:
> This use of `getGlobalIdentifier` (having a user-interface that relies on its format) seems novel. It warrants updating the Doxygen description of the function.
> 
> I believe some discussion should be had on LLVM Discourse about this.
I think clang::DeclarationName::getAsIdentifierInfo would be reasonable way to do it.  This is a facility for printing user readable names and it makes sense to me that it would be the string a user would expect to use when specifying the command line option.

If the variable isn't a simple name then it isn't eligible for toc-data.  

The more desirable way to to this would be a source-level attribute, which is something that ought to be considered in conjunction with this work, though I understand this is primarily to have an equivalent of the XL -`qdatalocal=` option for AIX.


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

https://reviews.llvm.org/D153907