[PATCH] D84610: [ELF] --icf: don't fold text sections with LSDA

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/ICF.cpp:524
+      if (sec->areRelocsRela)
+        handleLSDA(*part.ehFrame, *sec, sec->template relas<ELFT>());
+      else
----------------
MaskRay wrote:
> grimar wrote:
> > It's a bit strange to see that `eqClass[0]` is set to `xxHash64(s->data()` right above,
> > but `handleLSDA` might override the value to 1,2...N?
> > 
> > Should the hash be combined into, e.g. like `combineRelocHashes` does?
> Not necessary. By assigning 1,2,...,N, we don't actually want such sections to be folded into others. The combine operation is redundant.
> 
It is just strange to see that we drop the `xxHash64` work that was done, it looks excessive.

Right above we have the following code:

```
    auto *s = cast<InputSection>(sec);
    if (isEligible(s))
      sections.push_back(s);`
```

I've not checked, but have you considered something like not putting those sections into `sections` list at all?
I.e. make them non-eligible for ICF from the begining?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84610/new/

https://reviews.llvm.org/D84610