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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/Targets/PPC.cpp:274
+  if (auto *GVar = dyn_cast<llvm::GlobalVariable>(GV)) {
+    auto GVId = GVar->getGlobalIdentifier();
+
----------------
cebowleratibm wrote:
> 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.
`clang::DeclarationName::getAsIdentifierInfo` sounds like it would not take scopes and linkage into account. It also sounds like it has additional complications around macros.

Limiting the ability to specify specific symbols to cases of external linkage using the mangled symbol names is perhaps the least controversial in terms of having something that is solid (albeit restrictive).



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

https://reviews.llvm.org/D153907