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

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:317
+  /// True if we have a .lglobl directive. This is used to emit .lglobl .foo
+  /// for static function on AIX.
+  bool HasDotLGloblDirective = false;
----------------
```True if we have a .lglobl directive. This is used to emit .lglobl .foo for static function on AIX.```

Doesn't explain what .lglobal does. Instead I would suggest something along the lines of:

```True if we have a .lglobl directive, which is used to emit the information of a static symbol into the symbol table. Defaults to false.```


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:396
+  // If true, emit function descriptor symbol on AIX.
+  bool NeedsFuncDescSyntax = false;
+
----------------
nit: `NeedsFunctionDescriptors` instead?


================
Comment at: llvm/lib/MC/MCSectionXCOFF.cpp:27
+  default:
+    llvm_unreachable("Unsupported storage-mapping class");
+  }
----------------
A `report_fatal_error` is probably more appropriate until we have added all the storage mapping classes.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1693
+  const DataLayout &DL = getDataLayout();
+  unsigned Size = DL.getPointerSizeInBits() == 64 ? 8 : 4;
+
----------------
minor nit: `Size` --> `PointerSize`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66724