<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 3, 2014 at 10:35 PM, 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">

<div>Reid Kleckner wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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-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 inalloca args<br>
<br>
It disturbs the layout of the parameters in memory and registers,<br>
leading to problems in the backend.<br>
</blockquote>
<br></div>
:-(<div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The plan for optimizing internal inalloca functions going forward is to<br>
essentially SROA the argument memory and demote any captured arguments<br>
(things that aren't trivially written by a load or store) to an indirect<br>
pointer to a static alloca.<br>
<br>
Modified:<br>
     llvm/trunk/lib/Transforms/IPO/<u></u>DeadArgumentElimination.cpp<br>
     llvm/trunk/test/Transforms/<u></u>DeadArgElim/keepalive.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/<u></u>DeadArgumentElimination.cpp<br>
URL: <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>


==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Transforms/IPO/<u></u>DeadArgumentElimination.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/<u></u>DeadArgumentElimination.cpp 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 taken".<br>
  //<br>
  void DAE::SurveyFunction(const Function&F) {<br>
+  // Functions with inalloca parameters are expecting args in a particular<br>
+  // register and memory layout.<br>
+  if (F.getAttributes().<u></u>hasAttrSomewhere(Attribute::<u></u>InAlloca)) {<br>
+    MarkLive(F);<br>
</blockquote>
<br></div>
Hold on. Does it actually need to be live? Consider A(x) calls B(x) calls C(x). If C takes 'x' as inalloca and doesn't use it, B can pass in C(undef). If B doesn't take 'x' as inalloca, A doesn't need to pass B anything.</blockquote>

<div><br></div><div>No, it doesn't.  We could actually do the transform in this case if we changed the calling convention to something that doesn't have register parameters.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    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/<u></u>DeadArgElim/keepalive.ll<br>
URL: <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>


==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/test/Transforms/<u></u>DeadArgElim/keepalive.ll (original)<br>
+++ llvm/trunk/test/Transforms/<u></u>DeadArgElim/keepalive.ll Mon Feb  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 instead of<br>
+; memory.<br>
+define internal x86_thiscallcc i32 @unused_this(i32* %this, i32* inalloca %argmem) {<br>
</blockquote>
<br></div>
If it isn't used, please it readnone nocapture instead. Replacing it with 'undef' at the visible callsites would be nice too.<br></blockquote><div><br></div><div>That seems reasonable, but is it worth spending time on this instead of on argument promotion for inalloca?  That seems like a better way to enable DAE.</div>
<div> </div><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 have an internal x86_thiscallcc function instead of an internal fastcc function?</blockquote><div><br></div><div>Well, I reduced the crash from tablegen, so apparently we don't apply fastcc on everything internal?</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+       %v = load i32* %argmem<br>
+       ret i32 %v<br>
+}<br>
+; CHECK-LABEL: define internal x86_thiscallcc i32 @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* inalloca %m)<br>
+       ret i32 %v<br>
+}<br>
+<br>
  ; CHECK: attributes #0 = { nounwind }<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><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>
</blockquote>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><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>
</div></div></blockquote></div><br></div></div>