[PATCH] D68062: Propeller lld framework for basicblock sections

Sriraman Tallam via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 26 17:12:13 PDT 2019


On Thu, Sep 26, 2019 at 5:06 PM Han Shen via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> shenhan added a comment.
>
> In D68062#1683618 <https://reviews.llvm.org/D68062#1683618>, @ruiu wrote:
>
> > Let me start from very high-level review comments before getting into the details of the actual code:
> >
> > - First and foremost, can you add a file comments to each file to explain what you are doing in the file? It doesn't have to be detailed, but giving an overview at the beginning of a file is generally very useful.
>
>
> Thanks, Done.

This requires more comments, like a few lines before every function to
specifically describe what each function is doing. Let's iterate one
more time.

>
> > - As Michael pointed out in the RFC thread, we already have a similar feature (C3 ordering) in lld to order sections according to its graph structure. These two should probably be merged.
>
> Yup, that is reasonable. We discussed the issue this morning and think it is feasible to merge the C3 ordering w/ part of our reordering algorithm. However, we probably will delay this until after we get the first version in.
>
>
> Repository:
>   rLLD LLVM Linker
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68062/new/
>
> https://reviews.llvm.org/D68062
>
>
>


More information about the llvm-commits mailing list