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

[Nick Lewycky]

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

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

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?

> +	%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
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>