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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

> 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`.

I don't know if I understand what you said correctly, so let me write down my understanding.

- The main purpose of this patch is to make concurrent accesses to OutputOff and other members safe.
- The OuptutOff member currently races with Live member because they are bitfields of the same memory location.
- In order to make OutputOff race-free, we should make OutputOff a non-bitfield member.

Are we still on the same page? If so, please look at the following class.

  class SectionPiece {
  public:
    SectionPiece(size_t Off, uint32_t Hash, bool Live)
        : InputOff(Off), Live(Live || !Config->GcSections), Hash(Hash >> 1) {}
  
    uint64_t setOutputOff(uint64_t Val) {
      OutputOffHi = Val >> 32;
      OutputOffLo = Val;
    }
  
    uint64_t getOutputOff() const {
      return (uint64_t(OutputOffHi) << 32) | OutputOffLo;
    }
  
    uint32_t InputOff;
    uint32_t Hash;
    uint8_t TailHash;
    uint8_t Live;
  
  private:
    uint8_t OutputOffHi
    uint32_t OutputOffLo;
  };

Notice that {get,set}OutputOff don't race with accesses to InputOff/Hash/TailHash/Live. So the goal has achieved. Also with this scheme we don't sacrifice the bits of Hash -- Hash member still has the full 32 bits.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D60765