[llvm] r183625 - Make DeadArgumentElimination more conservative on variadic functions

[Tim Northover]

Author: tnorthover
Date: Sat Jun  8 21:17:27 2013
New Revision: 183625

URL: http://llvm.org/viewvc/llvm-project?rev=183625&view=rev
Log:
Make DeadArgumentElimination more conservative on variadic functions

Variadic functions are particularly fragile in the face of ABI changes, so this
limits how much the pass changes them

Added:
    llvm/trunk/test/Transforms/DeadArgElim/variadic_safety.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp

Modified: llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=183625&r1=183624&r2=183625&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp Sat Jun  8 21:17:27 2013
@@ -343,8 +343,9 @@ bool DAE::RemoveDeadArgumentsFromCallers
   if (Fn.isDeclaration() || Fn.mayBeOverridden())
     return false;
 
-  // Functions with local linkage should already have been handled.
-  if (Fn.hasLocalLinkage())
+  // Functions with local linkage should already have been handled, except the
+  // fragile (variadic) ones which we can improve here.
+  if (Fn.hasLocalLinkage() && !Fn.getFunctionType()->isVarArg())
     return false;
 
   if (Fn.use_empty())
@@ -604,9 +605,20 @@ void DAE::SurveyFunction(const Function
   UseVector MaybeLiveArgUses;
   for (Function::const_arg_iterator AI = F.arg_begin(),
        E = F.arg_end(); AI != E; ++AI, ++i) {
-    // See what the effect of this use is (recording any uses that cause
-    // MaybeLive in MaybeLiveArgUses).
-    Liveness Result = SurveyUses(AI, MaybeLiveArgUses);
+    Liveness Result;
+    if (F.getFunctionType()->isVarArg()) {
+      // Variadic functions will already have a va_arg function expanded inside
+      // them, making them potentially very sensitive to ABI changes resulting
+      // from removing arguments entirely, so don't. For example AArch64 handles
+      // register and stack HFAs very differently, and this is reflected in the
+      // IR which has already been generated.
+      Result = Live;
+    } else {
+      // See what the effect of this use is (recording any uses that cause
+      // MaybeLive in MaybeLiveArgUses). 
+      Result = SurveyUses(AI, MaybeLiveArgUses);
+    }
+
     // Mark the result.
     MarkValue(CreateArg(&F, i), Result, MaybeLiveArgUses);
     // Clear the vector again for the next iteration.

Added: llvm/trunk/test/Transforms/DeadArgElim/variadic_safety.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/variadic_safety.ll?rev=183625&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/DeadArgElim/variadic_safety.ll (added)
+++ llvm/trunk/test/Transforms/DeadArgElim/variadic_safety.ll Sat Jun  8 21:17:27 2013
@@ -0,0 +1,38 @@
+; RUN: opt < %s -deadargelim -S | FileCheck %s
+
+declare void @llvm.va_start(i8*)
+
+define internal i32 @va_func(i32 %a, i32 %b, ...) {
+  %valist = alloca i8
+  call void @llvm.va_start(i8* %valist)
+
+  ret i32 %b
+}
+
+; Function derived from AArch64 ABI, where 8 integer arguments go in
+; registers but the 9th goes on the stack. We really don't want to put
+; just 7 args in registers and then start on the stack since any
+; va_arg implementation already present in va_func won't be expecting
+; it.
+define i32 @call_va(i32 %in) {
+  %stacked = alloca i32
+  store i32 42, i32* %stacked
+  %res = call i32(i32, i32, ...)* @va_func(i32 %in, i32 %in, [6 x i32] undef, i32* byval %stacked)
+  ret i32 %res
+; CHECK: call i32 (i32, i32, ...)* @va_func(i32 undef, i32 %in, [6 x i32] undef, i32* byval %stacked)
+}
+
+define internal i32 @va_deadret_func(i32 %a, i32 %b, ...) {
+  %valist = alloca i8
+  call void @llvm.va_start(i8* %valist)
+
+  ret i32 %a
+}
+
+define void @call_deadret(i32 %in) {
+  %stacked = alloca i32
+  store i32 42, i32* %stacked
+  call i32 (i32, i32, ...)* @va_deadret_func(i32 undef, i32 %in, [6 x i32] undef, i32* byval %stacked)
+  ret void
+; CHECK: call void (i32, i32, ...)* @va_deadret_func(i32 undef, i32 undef, [6 x i32] undef, i32* byval %stacked)
+}