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. Repository: rL LLVM https://reviews.llvm.org/D36562 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
