[llvm] r200717 - inalloca: Don't remove dead arguments in the presence of inalloca args
Nick Lewycky
nicholas at mxc.ca
Tue Feb 4 00:08:12 PST 2014
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>
>
>
More information about the llvm-commits
mailing list