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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1972
 MCSection *TargetLoweringObjectFileXCOFF::getSectionForFunctionDescriptor(
-    const MCSymbol *FuncSym) const {
-  return getContext().getXCOFFSection(FuncSym->getName(), XCOFF::XMC_DS,
-                                      XCOFF::XTY_SD, XCOFF::C_HIDEXT,
-                                      SectionKind::getData());
+    const GlobalObject *GO, const MCSymbol *FuncSym) const {
+  return getContext().getXCOFFSection(
----------------
jasonliu wrote:
> MCSymbolXCOFF already have a getStorageClass() function, I don't think we need to pass in GlobalObject to get it. If we need to, then we get the wrong FuncSym.
we do not set StorageClass() for the FuncSym before we call the getSectionForFunctionDescriptor. We can not get getStorageClass() before set it.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1974
+  return getContext().getXCOFFSection(
+      FuncSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
+      getStorageClassForGlobal(GO), SectionKind::getData());
----------------
jasonliu wrote:
> FuncSym->getName() works means something is wrong here. 
> I assume FuncSym here is "foo", however FuncSym is supposed to be a "foo[DS]" name. 
> It's werid for us to have a symbol named "foo" here since we don't actually need that anywhere now after this patch. 
The FuncSym is symbol in the MCContext. not in the MCAssembler. it is foo[DS] in the MCAssembler (we put the foo[DS] into MCAssembler when we emiltLinkage ). we only care about MCAssembler in the XCOFFObjectWriter::executePostLayoutBinding()


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1549
+
+  if (XCOFFSym->hasContainingCsect()) {
+    MCSymbolXCOFF *QualName =
----------------
jasonliu wrote:
> This is similar issues I saw above, I think we have the wrong MCSymbol here. If we have the correct one, we could just call emitLinkage without this if statement.
when we do to the emitLinkage in the doFinalization()
there is function .llvm.StackProtector which do not have  hasContainingCsect()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76162