Xiangling_L marked 5 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:
> > > 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.

Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+  if (!isa<ConstantArray>(List))
+    return false;
jasonliu wrote:
> Return of boolean seems unnecessary. 
> Callee could check the size of the Structors to decide if they want an early 
> return or not (or in this particular case, the for loop would just do nothing 
> and no need for extra condition if you don't mind the call to 
> getPointerPrefAlignment or assign to 0 to Index)?
> So we could just return void for this function?
Yeah, we could do that. But it looks a boolean return value makes the code flow 
natural. And if any target does want to control the early return in the future, 
it's flexbile for them to do that. I am wondering is there any big difference 
between bool and void return value for this function? 



cfe-commits mailing list

Reply via email to