<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 1, 2011, at 2:13 PM, Ivan Krasin wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>this patch fixes the FastISel stack allocation strategy issue described here:<br><a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-July/041452.html">http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-July/041452.html</a><br>The solution is to emit immediate int arguments just before the call<br>(instead of spilling them on stack at the beginning of the function)<br></div></blockquote><br></div><div>Thanks for working on this, Ivan.</div><div><br></div><div>I don't think there is any reason to special-case function arguments. It is just as bad when fast-isel hoists the immediate in 'x+1'.</div><div><br></div><div>I would prefer an approach that handled all immediates.</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">--- test/CodeGen/X86/fast-isel-call-x86-64.ll<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</font></div><div><font class="Apple-style-span" color="#000000">+++ test/CodeGen/X86/fast-isel-call-x86-64.ll<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</font></div><div><font class="Apple-style-span" color="#000000">@@ -0,0 +1,59 @@</font></div><div><font class="Apple-style-span" color="#000000">+; RUN: llc < %s -O0 -fast-isel-abort -march=x86-64 | FileCheck %s</font></div><div><font class="Apple-style-span" color="#000000">+; This test should be merged with fast-isel-call.ll,</font></div><div><font class="Apple-style-span" color="#000000">+; but currently it's not possible because FastISel lacks</font></div><div><font class="Apple-style-span" color="#000000">+; of llvm.memcpy and llvm.memset.</font></div></blockquote><div><div><br></div></div><div>Huh? None of the tests use memcpy or memset.</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">+define i32 @test1() nounwind {</font></div><div><font class="Apple-style-span" color="#000000">+tak:</font></div><div><font class="Apple-style-span" color="#000000">+<span class="Apple-tab-span" style="white-space:pre"> </span>%tmp = call i1 @foo()</font></div><div><font class="Apple-style-span" color="#000000">+<span class="Apple-tab-span" style="white-space:pre"> </span>br i1 %tmp, label %BB1, label %BB2</font></div><div><font class="Apple-style-span" color="#000000">+BB1:</font></div><div><font class="Apple-style-span" color="#000000">+<span class="Apple-tab-span" style="white-space:pre"> </span>ret i32 1</font></div><div><font class="Apple-style-span" color="#000000">+BB2:</font></div><div><font class="Apple-style-span" color="#000000">+<span class="Apple-tab-span" style="white-space:pre"> </span>ret i32 0</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: test1:</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: callq</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK-NEXT: testb<span class="Apple-tab-span" style="white-space:pre"> </span>$1</font></div><div><font class="Apple-style-span" color="#000000">+}</font></div></blockquote><div><div><br></div></div><div>Please add a comment describing what you are testing for here.</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">+define void @test2(%struct.s* %d) nounwind {</font></div><div><font class="Apple-style-span" color="#000000">+ call void @foo2(%struct.s* byval %d )</font></div><div><font class="Apple-style-span" color="#000000">+ ret void</font></div><div><font class="Apple-style-span" color="#000000">+</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: test2:</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: subq<span class="Apple-tab-span" style="white-space:pre"> </span>$24, %rsp</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: movq<span class="Apple-tab-span" style="white-space:pre"> </span>(%rdi), %rax</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: movq<span class="Apple-tab-span" style="white-space:pre"> </span>%rax, (%rsp)</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: movl<span class="Apple-tab-span" style="white-space:pre"> </span>8(%rdi), %ecx</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: movl<span class="Apple-tab-span" style="white-space:pre"> </span>%ecx, 8(%rsp)</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: callq<span class="Apple-tab-span" style="white-space:pre"> </span>foo2</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: addq<span class="Apple-tab-span" style="white-space:pre"> </span>$24, %rsp</font></div><div><font class="Apple-style-span" color="#000000">+; CHECK: ret</font></div><div><font class="Apple-style-span" color="#000000">+}</font></div></blockquote><div><div><br></div></div><div>Same thing here. Also, is this related to your patch at all? Why must the byval argument be copied in that order and using exactly those temporary registers?</div><div><br></div><div>The last test looks good.</div><div><br></div><div>/jakob</div><div><br></div></div></div></div></body></html>