[PATCH] D66724: [AIX]Emit function descriptor csect in assembly

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:770
+void AsmPrinter::EmitFunctionDescriptor() {
+  report_fatal_error("Function descriptor is target-specific.");
+}
----------------
This could be done in the header instead. People could know it's a platform specific thing when reading the header, no need to jump to the source file.
Also I think we could use llvm_unreachable in this situation instead to get minor optimization advantage, but no big deal.  


================
Comment at: llvm/lib/MC/MCAsmInfoXCOFF.cpp:28
+
+bool MCAsmInfoXCOFF::isValidUnquotedName(StringRef Name) const { return true; }
----------------
We used isValidUnquotedName when printing symbol name in this context:
...
  StringRef Name = getName();
  if (!MAI || MAI->isValidUnquotedName(Name)) {
    OS << Name;
    return;
  }

  if (MAI && !MAI->supportsNameQuoting())
    report_fatal_error("Symbol name with unsupported characters");
...

If we always return true for isValidUnquotedName then we won't be able to detect symbol name with unsupported characters. 
On that note, we should probably set SupportsQuotedNames to be false for AIX target.
Using TOC[TC0] as symbol name is kinda a hack since XCOFF symbol could not contain '['  AFAIK. 
So maybe we would override isValidUnquotedName and make it very similar to the original one, except '[' is a valid Name temporarily. Add a FIXME to say remove this whole function when we don't use TOC[TC0] as symbol any more. 
 



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1739
+void PPCAIXAsmPrinter::EmitFunctionEntryLabel() {
+  return AsmPrinter::EmitFunctionEntryLabel();
+}
----------------
Minor nit: we don't need the "return".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66724