[PATCH] D37626: [ELF] Scan .eh_frame sections precisely in order to eliminate unused LSDAs and personality routines.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 6 inline comments as done.
ikudrin added a comment.

In https://reviews.llvm.org/D37626#882892, @ruiu wrote:

> How about this?
>
> 1. Add a new member, EhSectionPieces vector, to InputSection. It holds the input section's FDEs and CIEs if exists. At the beginning of markLive, scan all .eh_frame sections to initialize the vectors.
> 2. To represent edges between InputSections through .eh_frame section pieces, explicitly add these dependencies to InputSection::DependentSections. For example, if a function has .eh_frame pieces, its personality function is added to its DependentSection. You can initialize it at the beginning of markLive.
>
>   After doing the above two things, all dependencies are explicitly represented, and all you have to do is to traverse the graph, make .eh_frame pieces alive, and visit DependentSections as usual. I think this is a cleaner way of doing garbage-collecting.


I tried to implement this approach, see https://reviews.llvm.org/D38391. I hope I understood your suggestion right.

According to my tests, it is slower than the current version, approximately, by 10-20%. I checked it both with samples which use (like omnetpp) and do not use EH (like clang).



================
Comment at: ELF/MarkLive.cpp:165-166
+  const unsigned CieFirstRelI = Cie.FirstRelocation;
+  if (CieFirstRelI == (unsigned)-1)
+    return;
+
----------------
ruiu wrote:
> I still doubt if this check makes sense. If we visit an invalid CIE that doesn't point to anything and mark it alive, then that means we are going to emit an invalid CIE to the output file, no? It seems it is an error condition.
It's not a dangling or invalid CIE. It just doesn't refer to a personality function, which is not prohibited. For example, you can find such CIEs in any LLVM project on Linux, because they do not use exception handling.


================
Comment at: ELF/MarkLive.cpp:269-271
+  // The flag is set if an executable section is added to the queue.
+  // We scan .eh_frame sections only if the flag is set.
+  bool ExecInQueue = false;
----------------
ruiu wrote:
> Isn't this a premature optimization? I wouldn't add this unless it is proved to be actually effective.
I don't think so. It really helps to avoid the second scan of EH frames in most cases.


https://reviews.llvm.org/D37626