[PATCH] D60765: [ELF] Place SectionPiece::{Live,Hash} bit fields together

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D60765#1469806 <https://reviews.llvm.org/D60765#1469806>, @ruiu wrote:

> In D60765#1469717 <https://reviews.llvm.org/D60765#1469717>, @MaskRay wrote:
>
> >   uint32_t Hash;
> >   uint8_t Live : 1;
> >   uint8_t TailShardId : 5;
> >   uint8_t OutputOffHi;
> >   uint32_t OutputOffLo;
> >
> >
> > Do you mean
> >
> >   // 16 bytes -> 12 bytes
> >   uint32_t InputOff;
> >  
> >   uint32_t Live : 1
> >   uint32_t Hash : 18;
> >   uint32_t TailShardId : 5;
> >   uint32_t OutputOffHi : 8;
> >  
> >   uint32_t OutputOffLo;
> >
> >
> > Looks fine.
>
>
> Reducing the size to 12 bytes looks like a good idea, but I'm worried if 18 bits hash may be too short. This hash value will be used to determine a bucket in a hash table, so reducing the number of hash bits means that we'll likely to have more collisions.


It was my interpretation of your previous comment. I thought you wanted to do that (Now I think I don't understand what you were getting at):

> 40 bits for the output offset (should be large enough)

I am happy with the current 16-byte `SectionPiece`. Making `OutputOffHi` part of either `Hash` or `Live` can have data sharing caveats.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D60765