[PATCH] D84363: [XCOFF][AIX] Handle llvm.used and llvm.compiler.used global array

[Digger via Phabricator via llvm-commits]

DiggerLin accepted this revision.
DiggerLin marked an inline comment as done.
DiggerLin added a comment.

LGTM.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1682
+static bool isSpecialLLVMGlobalArrayToSkip(const GlobalVariable *GV) {
+  return GV->hasAppendingLinkage() &&
+         StringSwitch<bool>(GV->getName())
----------------
jasonliu wrote:
> jasonliu wrote:
> > DiggerLin wrote:
> > > jasonliu wrote:
> > > > DiggerLin wrote:
> > > > > not sure llvm.global_ctors must have an appending Linkage or not, I can not any information that talk about the llvm.global_ctors must having an appending Linkage.
> > > > > in the 
> > > > > https://llvm.org/docs/LangRef.html , I can find the description as "
> > > > > The @llvm.used global is an array which has appending linkage. "
> > > > > 
> > > > > and 
> > > > > "appending
> > > > > “appending” linkage may only be applied to global variables of pointer to array type. When two global variables with appending linkage are linked together, the two global arrays are appended together. This is the LLVM, typesafe, equivalent of having the system linker append together “sections” with identical names when .o files are linked"
> > > > > 
> > > > > but not for the llvm.global_ctors 
> > > > > 
> > > > > 
> > > > I did a search for llvm.global_ctors and llvm.global_dtors in clang's unit test, and found all the occurrence of those are with appending linkage. And in my rough look up in the code, I don't find anything that could generate those variable with another linkage. So I think it's safe to assume these special global variables all have appending linkage. I don't really see a reason for those special global variables not to have an appending linkage. If there is a potential reason for them to not have that linkage, I could remove this quick escape path.
> > > if you want  to make the code more efficient, the four strings has a common part "llvm." can we compare whether start with "llvm." , if not return false first, otherwise compare one by one?
> > We are still doing string comparison effectively, and you are not saving much by comparing the common part (unless we have more cases here, right now we have only three cases). And then you need to do all extra substr to compare the remaining character. That's not very appealing to me. 
> I added an assert below to make sure we know if there is something start with `llvm.` and we didn't handled it yet. That should address your concern.
> I think anything starts with "llvm." would need some special handling, so the assert could help us capture future new `llvm.` global variable, and existing `llvm.` that we don't know yet.
thanks


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

https://reviews.llvm.org/D84363