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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

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

> In D60765#1469826 <https://reviews.llvm.org/D60765#1469826>, @MaskRay wrote:
>
> > In D60765#1469825 <https://reviews.llvm.org/D60765#1469825>, @ruiu wrote:
> >
> > > The point of my last comment was that, with the new layout, accesses to OutputOffHi don't race with accesses to `Live` or `Hash`, as they are now in different non-integer-bitmap struct members.
> >
> >
> > I don't understand the split of `uint64_t OutputOff` -> `uint8_t OutputOffHi`+`uint32_t OutputOffLo` (40 bits).
> >
> >   uint32_t InputOff;
> >  
> >   uint32_t Hash;
> >   uint8_t Live : 1;
> >   uint8_t TailShardId : 5;
> >   uint8_t OutputOffHi;  // this may be written concurrently with the read of `Live` -> race
> >  
> >   uint32_t OutputOffLo;
> >
>
>
> Does that really race? `OutputOffHi` and `Live` are in different uint8_t in the above struct, so a write to `OutputOffHi` doesn't race with a read of `Live`, no?


It doesn't race. Since `OutputOffHi` is not a bitfield, it takes a different memory location. Concurrent accesses to different memory locations don't race. I still don't understand the split of `OutputOff`. If you pack other members adjacent to `OutputOffHi` and read them, it may race with the write of `OutputOffHi`.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D60765