[lld] [lld-macho,BalancedPartition] Simplify relocation hash and avoid xxHash (PR #121729)

Ellis Hoag via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 16 09:11:42 PST 2025


ellishg wrote:

> > > > > @ellishg Does the current PR look good to you?
> 
> > > > 
> 
> > > > 
> 
> > > > Can we use only `xxh3_64bits()` in this PR for simplicity and flexibility?
> 
> > > 
> 
> > > 
> 
> > > Do you want two PRs, one changing xxHash64 to `xxh3_64bits`, and the other simplifying the relocation hash?
> 
> > > If this is your intention, I can pre-land one commit to make just the xxHash64 change, and rebase this PR.
> 
> > 
> 
> > One PR should be ok. I was just saying we should use `xxh3_64bits()` instead of `read32le()` on line 100.
> 
> 
> 
> Using a hash function like `xxh3_64bits` for the k-mers introduces unnecessary performance overhead...
> 
> It's ok to have two sets of values into `hashes`: (a) instructions/data (b) `xxh3_64bits` derived values (relocations).
> 
> 
> 
> Well, for this PR to proceed I could give up if you are really strong about this.
> 
> But for the ELF port I'll ensure that we don't introduce this slight performance overhead...

I would guess the performance overhead of hashing is negligible compared to the runtime of the BP algorithm. I'm ok with this for now, but we should use the hashing algorithm if we ever find out that it can be beneficial to change the k-mer size (the number of bytes to hash).  

https://github.com/llvm/llvm-project/pull/121729


More information about the llvm-commits mailing list