[PATCH] D76162: [AIX] discard the label in the csect of function description and use qualname for linkage

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1974
+  SmallString<128> NameStr;
+  getMangler().getNameWithPrefix(NameStr, F, /*CannotUsePrivateLabel=*/false);
+  return getContext().getXCOFFSection(
----------------
DiggerLin wrote:
> jasonliu wrote:
> > Looking at what other XCOFF function is doing in this file, I think we should be consistent with them for easier maintenance which means we should call
> > 
> > ```
> > getNameWithPrefix(Name, GO, TM);
> > ```
> > instead. 
> TM is not a member of TargetLoweringObjectFile, do you want pass TM as parameter of getSectionForFunctionDescriptor() ? I do not think we need a TM as parameter of   getSectionForFunctionDescriptor.
I think it makes sense to pass it in. I understand right now calling to `getNameWithPrefix(Name, GO, TM)` is the same as what you have which is `getMangler()->getNameWithPrefix(OutName, GV, /*CannotUsePrivateLabel=*/false);`. But if later we decide to override `getNameWithPrefix(Name, GO, TM)` for XCOFF, we will forget there is this one place called directly to getMangler()->getNameWithPrefix .


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1740
+               getObjFileLowering().getSectionForFunctionDescriptor(
+                   dyn_cast<const Function>(GO)))
         ->getQualNameSymbol();
----------------
DiggerLin wrote:
> jasonliu wrote:
> > Why do we use dyn_cast? Is there a chance GO is not a Function when we gets here?
> getSectionForFunctionDescriptor(Function*) , but GO is GlobalObject.
Let me rephrase my question, could you use `cast` instead of `dyn_cast`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76162