[llvm-dev] Inlining + CSE + restrict pointers == funtimes

[Neil Henning via llvm-dev]

I've put the change up here https://reviews.llvm.org/D73342 for those
interested.

Cheers,
-Neil.

On Wed, Jan 22, 2020 at 5:13 PM Finkel, Hal J. <hfinkel at anl.gov> wrote:

> Thanks, Neil. To be clear, the patch should cause the intersect function
> to be called on the metadata.
>
>  -Hal
> On 1/22/20 11:09 AM, Neil Henning wrote:
>
> Ok I think we have some common ground - CSE should choose the aliased
> pointer over the non-aliased one because we don't want the no-aliasing
> information to creep outwards from the inlined callsite.
>
> I'll put together a patch in the coming days and add y'all as reviewers so
> you get visibility.
>
> Cheers,
> -Neil.
>
> On Wed, Jan 22, 2020 at 4:47 PM Jeroen Dobbelaere <
> Jeroen.Dobbelaere at synopsys.com> wrote:
>
>> That's indeed a good example that shows that dropping the metadata (A) is
>> the only correct solution.
>>
>>
>>
>> Greetings,
>>
>>
>>
>> Jeroen Dobbelaere
>>
>>
>>
>>
>>
>> *From:* Finkel, Hal J. <hfinkel at anl.gov>
>> *Sent:* Wednesday, January 22, 2020 17:33
>> *To:* Jeroen Dobbelaere <dobbel at synopsys.com>; Neil Henning <
>> neil.henning at unity3d.com>; llvm-dev at lists.llvm.org
>> *Subject:* Re: [llvm-dev] Inlining + CSE + restrict pointers == funtimes
>>
>>
>>
>> At a high level, EarlyCSE should be intersecting the metadata of
>> instructions that it combines. If it doesn't, and also doesn't drop the
>> metadata, that seems like a bug, regardless of anything else.
>>
>> On 1/22/20 9:30 AM, Jeroen Dobbelaere wrote:
>>
>> [..]
>>
>> -- (A) dropping the noalias information will result in less optimization
>> opportunities
>>
>> -- (B) preferring the load with the noalias annotation can allow more
>> reorderings and a better schedule.
>>
>>   (especially when there would be a 'store' in the context where the
>> restrict is valid; aka in 'called').
>>
>>
>>
>> My preference would go to (B) as that opens up more optimization
>> opportunities.
>>
>>
>>
>> I'm concerned that this isn't sound. So, imagine if I had something like
>> this:
>>
>> int * restrict r = a;
>> ...
>> int x = noaliasing ? *r : *a;
>>
>> then we need the aliasing load to retain its dependencies relative to
>> other things in the block. If we combine them into one load, it needs to be
>> the aliasing one.
>>
>> Thanks again,
>>
>> Hal
>>
>> [...]
>>
>>
>
> --
> Neil Henning
> Senior Software Engineer Compiler
> unity.com
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>

-- 
Neil Henning
Senior Software Engineer Compiler
unity.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200124/f1a4a062/attachment.html>