[PATCH] D44150: [WebAssembly] Use DenseMapInfo traits from LLVM repo. NFC

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added a comment.

In https://reviews.llvm.org/D44150#1037839, @ruiu wrote:

> > - ICF support (fold identical constant data sections together)
>
> For ICF, I'd recommend you make an improvement to the compiler and the wasm object file format first. The current implementation of lld's ICF is in some sense too aggressive -- it merges functions when they are identical. But that's actually a violation of the C/C++ spec because if you take addresses of two different functions, they must be distinct. If your program depends on pointer uniqueness, lld's ICF break your program.
>
> On the other hand, lld is conservative on merging data. Comparing two pointers for data is more common in programs, so we don't merge two read-only data by ICF even if their contents are identical. So there's a missed opportunity here.
>
> What we really should do is to make an improvement to the compiler so that it emits the information as to whether an address is significant or not for each chunk, and use that information in the linker. In this way, we can optimize linker output for size the most.


Cool! Good points.

Interestingly, for Wasm we //can// merge functions without violating pointer uniqueness. That's because we can create two different pointers for the exact same function! Function pointers are indexes into the "table", and the table in turn is populated with real function addresses. Currently we give each function one and only one pointer (so two aliases to the same function get the same pointer, as required), but if we merge two functions (via some "OutputFunction" object) then we'd still de-duplicate the table entries based on the InputFunctions, allowing one OutputFunction to have two pointers, and address-taken relocations would get the right value for the symbol.

I'll think about data too, that's a good point about needing to look at the relocations to work out whether the data is mergable.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44150