[polly] r299585 - Remove llvm.lifetime.start/end in original region.

[Michael Kruse via llvm-commits]

2017-04-06 5:51 GMT+02:00 Tobias Grosser via llvm-commits
<llvm-commits at lists.llvm.org>:
> On Wed, Apr 5, 2017, at 10:10 PM, Michael Kruse via llvm-commits wrote:
>> Author: meinersbur
>> Date: Wed Apr  5 15:09:59 2017
>> New Revision: 299585
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=299585&view=rev
>> Log:
>> Remove llvm.lifetime.start/end in original region.
>>
>> The current StackColoring algorithm does not correctly handle the
>> situation when some, but not all paths from a BB to the entry node
>> cross a llvm.lifetime.start. According to an interpretation of the
>> language reference at
>> http://llvm.org/docs/LangRef.html#llvm-lifetime-start-intrinsic
>> this might be correct, but it would cost too much effort to handle
>> in StackColoring.
>>
>> To be on the safe side, remove all lifetime markers even in the original
>> code version (they have never been copied to the optimized version)
>> to ensure that no path to the entry block will cross a
>> llvm.lifetime.start.
>>
>> The same principle applies to paths the a function return and the
>> llvm.lifetime.end marker, so we remove them as well.
>
> Hi Michael,
>
> I am not sure this is correct. What happens if there is on a different
> branch to the end node another lifetime marker outside of the region we
> are looking at? This marker will still be there, such that if we drop
> lifetime markers just from inside the region, there will be paths from
> the entry that either cross or do not cross start markers before they
> reach end.

This is described in the updated bug report, which I did not close for
this reason.

Removal of the llvm.lifetime.* markers was one of the possibilities
implied the llvm-dev thread I started. The other one was to sink/hoist
them: http://lists.llvm.org/pipermail/llvm-dev/2017-March/111669.html

Because the requirements for well-formed llvm.lifetime.* have not been
decided, we cannot make assumptions on what are correct lifetime
markers. clang never generates code with more than one
llvm.lifetime.start per pointer (CGDecl.cpp/CGExpr.cpp) and depending
on what the rules for correct lifetime marker placement are, it might
not even be well-formed. That is, this fixes the issue for 444.namd
and any code generated by clang. Until we get a proper validity
condition for lifetime markers, we don't really know what
transformations could be valid.

Moving the llvm.lifetime markers comes with the problem that the
pointer might not be defined at the point it is moved to. Also, one
had to ensure that there is no path with more than one
llvm.lifetime.start/end for the same pointer instance.

Removing llvm.lifetime.start/end globally comes with the problem that
we'd modify code in other regions we are not supposed to touch.

Michael