[llvm-dev] RFC: Adding argument allocas

[Hal Finkel via llvm-dev]

----- Original Message -----

> From: "Eli via llvm-dev Friedman" <llvm-dev at lists.llvm.org>
> To: "Reid Kleckner" <rnk at google.com>
> Cc: "llvm-dev" <llvm-dev at lists.llvm.org>
> Sent: Friday, December 9, 2016 2:21:57 PM
> Subject: Re: [llvm-dev] RFC: Adding argument allocas

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

This is a good point, but it does not seem complicated to implement. 

-Hal 

> > > 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-- 

Hal Finkel 
Lead, Compiler Technology and Programming Languages 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161210/4a69a971/attachment.html>