[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::equalsFde(EhSectionPiece *A, EhSectionPiece *B) {
+  if (!A || !B)
----------------
ruiu wrote:
> I doubt if this is correct. There may be relocations pointing to given FDEs, and I think you need to compare these relocations too.
This is correct I think, it just does not covers everything yet.
That is why I marked this patch as "first step" in description.

We need to scan relocations to compare LSDAs and also want to compare personality routines,
so want to take CIEs which contains a pointer to them in account when comparing FDEs during ICF.
That is what I want to implement after this patch.


================
Comment at: ELF/InputSection.h:330
 
+  EhSectionPiece *Fde = nullptr;
+
----------------
ruiu wrote:
> grimar wrote:
> > 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)
> I think I don't feel comfortable if you are going to check in a patch even though we know a correctness issue. Doing the right thing in this patch doesn't seem that hard.
Done. Added testcase.


https://reviews.llvm.org/D38319