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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/EhFrame.cpp:67
 
+bool elf::equalsFde(EhSectionPiece *A, EhSectionPiece *B) {
+  if (!A || !B)
----------------
I doubt if this is correct. There may be relocations pointing to given FDEs, and I think you need to compare these relocations too.


================
Comment at: ELF/InputSection.h:330
 
+  EhSectionPiece *Fde = nullptr;
+
----------------
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.


https://reviews.llvm.org/D38319