[llvm] r200717 - inalloca: Don't remove dead arguments in the presence of inalloca args
Reid Kleckner
rnk at google.com
Tue Feb 4 15:25:09 PST 2014
On Tue, Feb 4, 2014 at 12:08 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Reid Kleckner wrote:
>
>> On Mon, Feb 3, 2014 at 10:35 PM, Nick Lewycky <nicholas at mxc.ca
>> <mailto:nicholas at mxc.ca>> wrote:
>>
>> Reid Kleckner wrote:
>>
>> Author: rnk
>> Date: Mon Feb 3 14:42:49 2014
>> New Revision: 200717
>>
>> URL: http://llvm.org/viewvc/llvm- project?rev=200717&view=rev
>> <http://llvm.org/viewvc/llvm-project?rev=200717&view=rev>
>> Log:
>> inalloca: Don't remove dead arguments in the presence of
>> inalloca args
>>
>> It disturbs the layout of the parameters in memory and registers,
>> leading to problems in the backend.
>>
>>
>> :-(
>>
>>
>> The plan for optimizing internal inalloca functions going
>> forward is to
>> essentially SROA the argument memory and demote any captured
>> arguments
>> (things that aren't trivially written by a load or store) to an
>> indirect
>> pointer to a static alloca.
>>
>> Modified:
>> llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp
>> llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/
>> DeadArgumentElimination.cpp
>> URL: http://llvm.org/viewvc/llvm- project/llvm/trunk/lib/
>> Transforms/IPO/ DeadArgumentElimination.cpp?
>> rev=200717&r1=200716&r2= 200717&view=diff
>> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Transforms/IPO/DeadArgumentElimination.cpp?rev=200717&r1=200716&r2=
>> 200717&view=diff>
>> ============================== ==============================
>> ==================
>> --- llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/IPO/ DeadArgumentElimination.cpp
>>
>> Mon Feb 3 14:42:49 2014
>> @@ -531,6 +531,13 @@ DAE::Liveness DAE::SurveyUses(const Valu
>> // well as arguments to functions which have their "address
>> taken".
>> //
>> void DAE::SurveyFunction(const Function&F) {
>> + // Functions with inalloca parameters are expecting args in a
>> particular
>> + // register and memory layout.
>> + if (F.getAttributes(). hasAttrSomewhere(Attribute:: InAlloca))
>> {
>>
>> + MarkLive(F);
>>
>>
>> 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.
>>
>>
>> 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.
>>
>> + return;
>> + }
>> +
>> unsigned RetCount = NumRetVals(&F);
>> // Assume all return values are dead
>> typedef SmallVector<Liveness, 5> RetVals;
>>
>> Modified: llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll
>> URL: http://llvm.org/viewvc/llvm- project/llvm/trunk/test/
>> Transforms/DeadArgElim/ keepalive.ll?rev=200717&r1=
>> 200716&r2=200717&view=diff
>> <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>> Transforms/DeadArgElim/keepalive.ll?rev=200717&r1=
>> 200716&r2=200717&view=diff>
>> ============================== ==============================
>> ==================
>> --- llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll
>> (original)
>> +++ llvm/trunk/test/Transforms/ DeadArgElim/keepalive.ll Mon Feb
>>
>> 3 14:42:49 2014
>> @@ -28,4 +28,20 @@ define void @caller() {
>> ret void
>> }
>>
>> +; We can't remove 'this' here, as that would put argmem in ecx
>> instead of
>> +; memory.
>> +define internal x86_thiscallcc i32 @unused_this(i32* %this,
>> i32* inalloca %argmem) {
>>
>>
>> If it isn't used, please it readnone nocapture instead. Replacing it
>> with 'undef' at the visible callsites would be nice too.
>>
>>
>> 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.
>
>
> 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.
>
> 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.
>
>
>>
>> + %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/f0e81dc2/attachment.html>
More information about the llvm-commits
mailing list