[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

[Hal Finkel via llvm-commits]

On 08/09/2017 10:37 PM, Hal Finkel via llvm-commits wrote:
>
>
> On 08/09/2017 10:14 PM, Xinliang David Li via llvm-commits wrote:
>>
>>
>> On Wed, Aug 9, 2017 at 7:42 PM, Chandler Carruth via Phabricator 
>> <reviews at reviews.llvm.org <mailto: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?

Also, on this, unfortunately, no. The problem is that the optimization 
would be tested using a specific regression test. That test will 
continue to pass even if you change the IR that frontend generates so 
that the test no longer represents the partially-optimized IR pattern 
that the frontend will help create. This is why our tendency to rely 
only on pass-specific regression tests, and not having end-to-end tests, 
is not necessarily optimal.

  -Hal

>> And what information is missing?
>
> To make a general statement, if we load (a, i8) and (a+2, i16), for 
> example, and these came from some structure, we've lost the 
> information that the load (a+1, i8) would have been legal (i.e. is 
> known to be deferenceable). This is not specific to bit fields, but 
> the fact that we lose information on the dereferenceable byte ranges 
> around memory access turns into a problem when we later can't legally 
> widen. There may be a better way to keep this information other than 
> producing wide loads (which is an imperfect mechanism, especially the 
> way we do it by restricting to legal integer types), but at the 
> moment, we don't have anything better.
>
>  -Hal
>
>>
>> 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 <https://reviews.llvm.org/D36562>
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> -- 
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170809/45e18f06/attachment.html>