[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 11:01 PM, Xinliang David Li wrote:
>
>
> On Wed, Aug 9, 2017 at 8:44 PM, Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>> wrote:
>
>
>     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.
>
>
> If widening is expected in the result, the end state IR can still be 
> checked without the need for end-end testing.

I don't understand what you mean. Exactly what test do you think we 
would have?

>     This is why our tendency to rely only on pass-specific regression
>     tests, and not having end-to-end tests, is not necessarily optimal.
>
>
> End-end tests still exist, especially performance regression tests -- 
> missing optimizations will likely result in  performance regressions 
> in real world applications, or else it probably does not matter :)

Yes. However, we have very few performance regression tests. There are a 
lot of real-world applications, many of which have different dynamic 
states in which different routines are important, many more 
configurations than any of us can ever benchmark, profile, or analyze. 
So we do what we can on that front, and then we rely on consistency, 
generality, and robustness to hopefully cover all of the rest of them. 
Professional judgment comes in when we weigh the costs of that 
generality, robustness, and consistency vs. the costs of development.

  -Hal

>
> David
>
>      -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 <mailto:llvm-commits at lists.llvm.org>
>>>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>     <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 <mailto:llvm-commits at lists.llvm.org>
>>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>     <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
>     -- 
>     Hal Finkel
>     Lead, Compiler Technology and Programming Languages
>     Leadership Computing Facility
>     Argonne National Laboratory
>
-- 
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/28214dc6/attachment.html>