<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 12/9/2016 11:49 AM, Reid Kleckner
wrote:<br>
</div>
<blockquote
cite="mid:CACs=tyLhNBrgO3d9WgBPLyxwt+RdkbhoiyWYwLXio4wnfWQSxw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On Fri, Dec 9, 2016 at 10:30 AM,
Friedman, Eli <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><span
class="gmail-m_8999134631902946825m_4643703738770916263gmail-">
<div
class="gmail-m_8999134631902946825m_4643703738770916263gmail-m_-1365877156293351287moz-cite-prefix">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.</div>
</span></div>
</blockquote>
<div><br>
</div>
<div>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::FindAvailableLoaded<wbr>Value, which looks for
StoreInst but not AllocaInst.</div>
</div>
</div>
</div>
</blockquote>
<br>
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:<br>
<br>
void g(int *x);<br>
__attribute((noinline)) void f(int y) {<br>
y = 10;<br>
g(&y);<br>
}<br>
void h() { f(134314); }<br>
<br>
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".<br>
<br>
<blockquote
cite="mid:CACs=tyLhNBrgO3d9WgBPLyxwt+RdkbhoiyWYwLXio4wnfWQSxw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"> 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.</div>
</blockquote>
<div><br>
</div>
<div>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: <a moz-do-not-send="true"
href="https://llvm.org/bugs/show_bug.cgi?id=21573">https://llvm.org/bugs/show_bug.cgi?id=21573</a></div>
</div>
</div>
</div>
</blockquote>
<br>
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.<br>
<p>-Eli<br>
</p>
<pre class="moz-signature" cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
</body>
</html>