[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
 
+static bool equalsFde(EhSectionPiece *A, EhSectionPiece *B) {
+  // We want to compare FDEs contents, for that we want to ignore
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > grimar wrote:
> > > > ruiu wrote:
> > > > > grimar wrote:
> > > > > > ruiu wrote:
> > > > > > > I doubt if this is correct because this function seems to compare contents only byte-by-byte. You need to handle relocations right?
> > > > > > I have already answered the same question here:
> > > > > > https://reviews.llvm.org/D38319#inline-335138
> > > > > That URL just jumps to the top of the page to me.
> > > > Probably becaue this comment is hidden for you in phab view ? It was Mon, Oct 2, 1:39 PM:
> > > > 
> > > > "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."
> > > > 
> > > I'd like to know how you'd scan relocations. If you have a patch, can you upload?
> > Not yet. Can try to do it and show you.
> It's worth doing. When I implement a feature, I often do it several times locally to see what is the best way of doing it.
I would like to clarify, do you think we can go in that direction for now (and then I'll try to update this with relocations scanning) or we would like to put this on hold for a while since honestly it is not clear for me with what "[RFC] Making .eh_frame more linker-friendly" can end up ?


https://reviews.llvm.org/D38319