[llvm] r200717 - inalloca: Don't remove dead arguments in the presence of inalloca args

[Reid Kleckner]

On Tue, Feb 4, 2014 at 12:08 AM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Reid Kleckner wrote:
>
>> 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.
>>
>
> 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.


I think so.  "Guarantee" sounds a bit strong.  :)

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.

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.


>      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?
>>
>>
>> Well, I reduced the crash from tablegen, so apparently we don't apply
>> fastcc on everything internal?
>>
>
> 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.
>

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.


> 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.
>

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.

I'll make a patch to do this.


>
>>
>>         +       %v = load i32* %argmem
>>         +       ret i32 %v
>>         +}
>>         +; CHECK-LABEL: define internal x86_thiscallcc i32
>>         @unused_this(i32* %this, i32* inalloca %argmem)
>>         +
>>         +define i32 @caller2() {
>>         +       %t = alloca i32
>>         +       %m = alloca i32, inalloca
>>         +       store i32 42, i32* %m
>>         +       %v = call x86_thiscallcc i32 @unused_this(i32* %t, i32*
>>         inalloca %m)
>>         +       ret i32 %v
>>         +}
>>         +
>>            ; CHECK: attributes #0 = { nounwind }
>>
>>
>>         ______________________________ _________________
>>         llvm-commits mailing list
>>         llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>         http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
>>         <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>     ______________________________ _________________
>>     llvm-commits mailing list
>>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
>>     <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140204/cdf1b4e7/attachment.html>