[llvm-dev] RFC: Adding argument allocas

[Friedman, Eli via llvm-dev]

On 12/9/2016 11:49 AM, Reid Kleckner wrote:
> On Fri, Dec 9, 2016 at 10:30 AM, Friedman, Eli 
> <efriedma at codeaurora.org <mailto:efriedma at codeaurora.org>> wrote:
>
>     Mmm... maybe.  The part I really don't like is the implied store:
>     there are a lot of transformations which specifically reason about
>     store instructions, and they would all need to be fixed to
>     specifically deal with "alloca with an argument" so it doesn't
>     block optimizations.  Every feature we add to the IR makes it more
>     difficult to write IR transformations, and this really doesn't
>     seem to carry its own weight.
>
>
> I don't feel like it complicates our model that much, though. Passes 
> already have to know that uninitialized allocas contain undef, and 
> with this change they can contain a value. That doesn't seem very 
> surprising. It is a fair point that we'd have to add new cases to code 
> like llvm::FindAvailableLoadedValue, which looks for StoreInst but not 
> AllocaInst.

It's more that that... I mean, obviously you have to fix all the places 
which assume allocas start off containing undef, but you're also 
introducing the potential for missed optimizations by making the store 
implicit.  For example, consider:

void g(int *x);
__attribute((noinline)) void f(int y) {
   y = 10;
   g(&y);
}
void h() { f(134314); }

The argument to f is dead; there are two stores to y, and instcombine 
will eliminate the first one.  With extended allocas, we need a new form 
of dead store elimination which transforms "alloca i32, argument %x" -> 
"alloca i32".

>     2. We could change the way clang generates function definitions:
>     instead of "define void @f(i32 %x)", generate "define void @f(i32*
>     byval %px)" if the value is going to end up on the stack.  This
>     should work without any changes to the IR.  This is a bad idea if
>     we're trying to run optimizations because it obscures data flow,
>     but it seems reasonable at -O0.
>
>
> This is problematic because it would break out "bitcode ABI" between 
> optimized and debug builds. We've had problems in the past when the 
> caller and callee disagree on whether an argument is a byval pointer 
> or not, and instcombine will come along and try to simplify the casts: 
> https://llvm.org/bugs/show_bug.cgi?id=21573

If instcombine is breaking this somehow, it's a bug; we're not supposed 
to eliminate bitcasts unless we can prove the signatures are 
equivalent.  Granted, this is a complicated transform which has had a 
lot of bugs in the past.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161209/6923999d/attachment.html>