[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Xinliang David Li via llvm-commits]

On Wed, Aug 9, 2017 at 7:42 PM, Chandler Carruth via Phabricator <
reviews at reviews.llvm.org> wrote:

> chandlerc added a comment.
>
> 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.
>
>

Can you elaborate on this? Do you mean race conditions due to widening?



> 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.
>
>
 Can you elaborate here too? If there were missed optimization that later
got fixed, there should be regression tests for them, right?  And what
information is missing?

thanks,

David




> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170809/3975a50c/attachment.html>