[PATCH] D53408: [PPC64] Long branch thunks.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:3086
+bool PPC64LongBranchTargetSection::empty() const {
+  return Finalized && Entries.size() == 0;
+}
----------------
MaskRay wrote:
> MaskRay wrote:
> > `Entries.size() == 0` -> `Entries.empty()`
> A synthetic section's `empty()` function is called twice:
> 
> * In `removeUnusedSyntheticSections()`
>    It should return false so that an empty `.branch_lt` will not be removed.
>    I find that empty `.branch_lt` is kept in both ld.bfd and gold, so the behavior is consistent with them. But I think it deserves a comment.
> * In `applySynthetic` (called by `finalizeSections()`)
>   This is to finalize a section which will assign true to `Finalized`.
> 
> In both cases, the section has not been finalized and `Finalized` is false, thus the function will return `false` in both cases.
> 
I originally just had `empty()` always return false but thought there may be code in the future that comes after thunk allocation that wants to skip any remaining empty sections. I also considered removing the section after thunk allocation if it was still empty, but decided that the added complexity wasn't worth it. 


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53408