[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+                              SmallVector<Structor, 8> &Structors);
----------------
jasonliu wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > Xiangling_L wrote:
> > > > jasonliu wrote:
> > > > > A doxygen comment describe what this function does, and what its return value means, and mention `Structors` is an output argument.
> > > > > By looking at what this function does, it seems `buildStructorList` is a better name.
> > > > I meant to and should've named this function to `preprocessXXStructorList`, let me know if you still prefer `buildStructorList`. And if you do, since the underneath of `SmallVector` is a variable-sized array, maybe we should try `buildSortedStructorArray`?
> > > `preprocess` sounds like we are already having a XXStructorList and now we try to do something on it. 
> > > But right now, we are actually passing in an empty StructorList/Array and build it from scratch. So I would still prefer the name of `build` in it.
> > > I don't mind changing to a more accurate name as you suggested. 
> > I think we do have a `XXStructorList` here which is the initializer of `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is  consistent with other spots.
> My understanding is that before we enter this `preprocessXXStructorList`, we do not have a list of XXStructor. We only have a list of `Constant`. This functions turns a list of `Constant` to a list of `XXStructor`. 
Just leave a record here, as we discussed offline, we agree that the meaning of term `XXStructorList` is `the initializer of `llvm.gloal_ctors` or `llvm.gloal_dtors` array. So I will keep using `preprocessXXStructorList` as the function name.


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

https://reviews.llvm.org/D84534