<div dir="ltr"><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 4, 2014 at 12:08 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Reid Kleckner wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On Mon, Feb 3, 2014 at 10:35 PM, Nick Lewycky <<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a><br></div><div><div class="h5">
<mailto:<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>>> wrote:<br>
<br>
Reid Kleckner wrote:<br>
<br>
Author: rnk<br>
Date: Mon Feb 3 14:42:49 2014<br>
New Revision: 200717<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-" target="_blank">http://llvm.org/viewvc/llvm-</a> project?rev=200717&view=rev<br>
<<a href="http://llvm.org/viewvc/llvm-project?rev=200717&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=200717&view=rev</a>><br>
Log:<br>
inalloca: Don't remove dead arguments in the presence of<br>
inalloca args<br>
<br>
It disturbs the layout of the parameters in memory and registers,<br>
leading to problems in the backend.<br>
<br>
<br>
:-(<br>
<br>
<br>
The plan for optimizing internal inalloca functions going<br>
forward is to<br>
essentially SROA the argument memory and demote any captured<br>
arguments<br>
(things that aren't trivially written by a load or store) to an<br>
indirect<br>
pointer to a static alloca.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp<br>
llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-" target="_blank">http://llvm.org/viewvc/llvm-</a> project/llvm/trunk/lib/<br>
Transforms/IPO/ DeadArgumentElimination.cpp?<br>
rev=200717&r1=200716&r2= 200717&view=diff<br></div></div>
<<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=200717&r1=200716&r2=200717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Transforms/IPO/<u></u>DeadArgumentElimination.cpp?<u></u>rev=200717&r1=200716&r2=<u></u>200717&view=diff</a>><br>
============================== ==============================<br>
==================<br>
--- llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp<br>
(original)<br>
+++ llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp<div class="im"><br>
Mon Feb 3 14:42:49 2014<br>
@@ -531,6 +531,13 @@ DAE::Liveness DAE::SurveyUses(const Valu<br>
// well as arguments to functions which have their "address<br>
taken".<br>
//<br>
void DAE::SurveyFunction(const Function&F) {<br>
+ // Functions with inalloca parameters are expecting args in a<br>
particular<br>
+ // register and memory layout.<br></div>
+ if (F.getAttributes(). hasAttrSomewhere(Attribute:: InAlloca)) {<div class="im"><br>
+ MarkLive(F);<br>
<br>
<br>
Hold on. Does it actually need to be live? Consider A(x) calls B(x)<br>
calls C(x). If C takes 'x' as inalloca and doesn't use it, B can<br>
pass in C(undef). If B doesn't take 'x' as inalloca, A doesn't need<br>
to pass B anything.<br>
<br>
<br>
No, it doesn't. We could actually do the transform in this case if we<br>
changed the calling convention to something that doesn't have register<br>
parameters.<br>
<br>
+ return;<br>
+ }<br>
+<br>
unsigned RetCount = NumRetVals(&F);<br>
// Assume all return values are dead<br>
typedef SmallVector<Liveness, 5> RetVals;<br>
<br>
Modified: llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-" target="_blank">http://llvm.org/viewvc/llvm-</a> project/llvm/trunk/test/<br>
Transforms/DeadArgElim/ keepalive.ll?rev=200717&r1=<br>
200716&r2=200717&view=diff<br></div>
<<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/keepalive.ll?rev=200717&r1=200716&r2=200717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/test/<u></u>Transforms/DeadArgElim/<u></u>keepalive.ll?rev=200717&r1=<u></u>200716&r2=200717&view=diff</a>><br>
============================== ==============================<br>
==================<br>
--- llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll (original)<br>
+++ llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll Mon Feb<div class="im"><br>
3 14:42:49 2014<br>
@@ -28,4 +28,20 @@ define void @caller() {<br>
ret void<br>
}<br>
<br>
+; We can't remove 'this' here, as that would put argmem in ecx<br>
instead of<br>
+; memory.<br>
+define internal x86_thiscallcc i32 @unused_this(i32* %this,<br>
i32* inalloca %argmem) {<br>
<br>
<br>
If it isn't used, please it readnone nocapture instead. Replacing it<br>
with 'undef' at the visible callsites would be nice too.<br>
<br>
<br>
That seems reasonable, but is it worth spending time on this instead of<br>
on argument promotion for inalloca? That seems like a better way to<br>
enable DAE.<br>
</div></blockquote>
<br>
Are you guaranteeing that argument promotion will fire for all cases where DAE would? Note that this DAE handles values threaded through arbitrarily complex argument/return paths throughout a module.<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, is this a representative case in the real world? Why do you<br>
have an internal x86_thiscallcc function instead of an internal<br>
fastcc function?<br>
<br>
<br>
Well, I reduced the crash from tablegen, so apparently we don't apply<br>
fastcc on everything internal?<br>
</blockquote>
<br></div>
Please investigate that. Firstly, do you want to change the CC? It will break anybody who expects this particular CC. That might mean debuggers, that might mean exceptions. I don't know what else.<br>
<br>
Assuming you do, the safety checks test whether the calling convention is the default one (GlobalOpt.cpp:1915). I can't think of a reason for that safety check at all, but if there is one, you can factor this into a function that tests any number of calling conventions.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<br>
<br>
+ %v = load i32* %argmem<br>
+ ret i32 %v<br>
+}<br>
+; CHECK-LABEL: define internal x86_thiscallcc i32<br>
@unused_this(i32* %this, i32* inalloca %argmem)<br>
+<br>
+define i32 @caller2() {<br>
+ %t = alloca i32<br>
+ %m = alloca i32, inalloca<br>
+ store i32 42, i32* %m<br>
+ %v = call x86_thiscallcc i32 @unused_this(i32* %t, i32*<br>
inalloca %m)<br>
+ ret i32 %v<br>
+}<br>
+<br>
; CHECK: attributes #0 = { nounwind }<br>
<br>
<br>
______________________________ _________________<br>
llvm-commits mailing list<br></div>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/" target="_blank">http://lists.cs.uiuc.edu/</a> mailman/listinfo/llvm-commits<br>
<<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a>><br>
<br>
<br>
______________________________ _________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/" target="_blank">http://lists.cs.uiuc.edu/</a> mailman/listinfo/llvm-commits<br>
<<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a>><br>
<br>
<br>
</blockquote>
<br>
</blockquote></div><br></div>