[PATCH] D38319: [ELF] - Teach ICF to take FDEs into account when doing code folding.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/EhFrame.cpp:67
 
+bool elf::areFdesEqual(EhSectionPiece *A, EhSectionPiece *B) {
+  if (!A || !B)
----------------
ruiu wrote:
> I wonder if you actually tried this.
> 
> Do you really have to care about padding?
Yes, please see `eh-frame-hdr-icf-fde1.s` testcase from this patch, it shows that 2 FDEs differs because of padding only.


================
Comment at: ELF/ICF.cpp:285
 
+  if (!areFdesEqual(A->Fde, B->Fde))
+    return false;
----------------
ruiu wrote:
> Rename equalsFde
Done.


================
Comment at: ELF/InputSection.h:330
 
+  EhSectionPiece *Fde = nullptr;
+
----------------
ruiu wrote:
> I strongly doubt if this is correct because the relationship between input sections and fdes is 1 to many.
That is sure needs to be vector, but for start I decided to support simple case and improve it in following patches.
This patch open roads for other patches teaching ICF about handling .eh_frame data and it is already large (has 3 new testcases),
so to reduce amount of changes I supposed we can start from this ?

(FWIW having it as the simple pointer now does not break anything for us because before this patch ICF merged identical code ignoring all FDEs, but it already fixes some cases at the same time)


https://reviews.llvm.org/D38319