wmi added a comment.
In https://reviews.llvm.org/D36562#837594, @chandlerc wrote:
> This has been discussed before and I still pretty strongly disagree with it.
> This cripples the ability of TSan to find race conditions between accesses to
> consecutive bitfields -- and these bugs have actually come up.
I guess you mean accessing different bitfields in a consecutive run
simultaneously can cause data race. Because bitfields order in a consecutive
run is implementation defined. With the change, Tsan may miss reporting that,
but such data race can be exposed in a different compiler.
This can be solved by detecting tsan mode in the code. If tsan is enabled, we
can stop splitting the bitfields.
> We also have had cases in the past where LLVM missed significant bitfield
> combines and optimizations due to loading them as separate integers. Those
> would become problems again, and I think they would be even harder to solve
> than narrowing the access is going to be because we will have strictly less
> information to work with.
how about only separating legal integer width bitfields at the beginning of a
consecutive run? Then it won't hinder bitfields combines. This way, it can
still help for some cases, including the important case we saw.
> Ultimately, while I understand the appeal of this approach, I don't think it
> is correct and I think we should instead figure out how to optimize these
> memory accesses well in LLVM. That approach will have the added benefit of
> optimizing cases where the user has manually used a large integer to simulate
> bitfields, and making combining and canonicalization substantially easier.
cfe-commits mailing list