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

[Xinliang David Li via llvm-dev]

On Sun, Jul 31, 2016 at 9:47 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:
>

There is one caveat: stop doing stack merging in inliner will change the
inliner cost analysis which may have affect inlining decisions and
performance in an unexpected way.  For instance, the allocatedSize estimate
won't be as accurate due to double counting.


> 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.
>

The code handling the merging seems to be in an isolated place with <100
lines of code. While I agree this change may be something good to do in
principle -- are there any practical reason for doing this? you mentioned
information loss (aliasing, sanitizer etc) -- are there any real issues
caused by the merging (with tracking bugs)?

David



> 5) Profit!
>
> -Chandler
>
> _______________________________________________
> 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/96c37e7e/attachment.html>