[llvm-dev] RFC: We should stop merging allocas in the inliner

[Daniel Berlin via llvm-dev]

The existing lifetime start/end is not very well defined (by spec, or by
code) in what it means, so you could have nested lifetime markers if you
wanted.   If you made the spec well-defined, they would be meaningful (for
loops, etc).

There are a number of open bugs/complaints about lifetime markers and the
fact that the scope is not well defined (the spec says "This intrinsic
indicates that before this point in the code, the value of the memory
pointed to by ptr is dead. This means that it is known to never be used and
has an undefined value. A load from the pointer that precedes this
intrinsic can be replaced with 'undef'.)

"Before" is not a well-defined term :)  (the end code says after, which has
similar problems)

Trivial case:

Two block loop consists, lifetime maker at beginning

A
|  ^
|    \
|    |
|   |
v /
B

Lifetime marker is at top of A.

Is B before A? (if not, are we using dominates as the definition of
"before", because B is a predecessor of A)
Does the answer change if i put a jump to B somewhere above A?
If we use dominates, and i put a jump there, i can make A and B dominator
tree siblings.  Is the pointer now invalid? :)

etc

I suspect what is really wanted is "i want lifetime markers to specify that
this thing has single-iteration scope, function scope, or SESE/SEME region
scope"  (or something).



On Mon, Aug 1, 2016 at 8:36 AM, David Blaikie via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Would we want to wrap any of the allocas we do import while inlining in
> extra lifetime markers? (not sure if nested lifetime markers are
> supported/meaningful - maybe just in the case where there are no lifetime
> markers already?) So that non-Clang IR clients still get stack reuse for
> inlined stack slots?
>

> On Sun, Jul 31, 2016 at 9:48 PM Chandler Carruth via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Chris added alloca merging in the inliner a looooong time ago, 2009. The
>> reason he added it was because at the time we didn't do stack coloring and
>> without it we had serious stack size problems in LLVM.
>>
>> Since then, a few rather important things have changed:
>> - We have reasonably powerful stack coloring support in the backend based
>> on lifetime markers
>> - Clang (the primary frontend I'm aware of that hit issues with this) is
>> now emitting lifetime markers for essentially everything
>> - Clang even has front-end based -O0 stack reuse tricks
>> - AliasAnalysis has become even more important (extended to codegen time
>> etc) and relies heavily on distinguishing between allocas.
>> - We have lots of tools like the sanitizers that directly reason about
>> allocas to catch bugs in user programs.
>>
>> The first two changes pretty much completely obviate the need for alloca
>> merging as far as I'm aware, and the second two changes make the
>> information loss caused by this practice, IMO, really bad.
>>
>> Even if there are problems exposed by turning off alloca merging in the
>> inliner, they are almost certainly deficiencies in stack coloring, Clang,
>> or other parts of the optimizer that we should fix rather than continue to
>> ignore.
>>
>> Thoughts? The code changes are easy and mechanical. My plan would be:
>> 1) Add a flag to turn this off.
>> 2) Run benchmarks with flag to make sure things are actually working.
>> 3) Change default for the flag, make sure chaos doesn't ensue.
>> 4) Delete the (quite substantial) complexity associated with this and the
>> flag.
>> 5) Profit!
>>
>> -Chandler
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160801/59328287/attachment.html>