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

Reply via email to