<div dir="ltr"><div class="gmail_extra"><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">That seems reasonable, but is it worth spending time on this instead of<br></div><div class="im">
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.</blockquote><div><br>
</div><div>I think so.  "Guarantee" sounds a bit strong.  :)</div><div><br></div><div>DAE only fires on internal functions, which is the same kind of thing we can promote to eliminate inalloca, even if a constructor captures the address of one of the fields.  We can define the semantics of inalloca allocas such that the user isn't allowed to GEP from the address of a field captured by a constructor to the next or previous argument.</div>
<div><br></div><div>Then DAE can do its thing in peace without having to handle the complexity of inalloca.  I'm not sure if the phase ordering is right today, but that's how I'd like to make this work.</div><div>
 </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<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></blockquote><div>
<br></div><div>I would say we can probably change the CC of an internal thiscall function.  Debugging on Windows is a bridge we'll have to cross later.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

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>
</blockquote><div><br></div><div>I can see us not wanting to change the CC of something that is coldcc, for example.  It also seems a bit hostile to change the CC of things the user explicitly requested calling conventions for.  It seems like we want a predicate for, "was this probably an implicitly applied calling convention?", in which case, we should transform away.</div>
<div><br></div><div>I'll make a patch to do this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></div>