From: Tim Northover <tnorthover@apple.com>
Date: Sun, 9 Jun 2013 02:17:27 +0000 (+0000)
Subject: Make DeadArgumentElimination more conservative on variadic functions
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=d84b17e157bb27bed236a400cccf4562d0b19d96;p=oota-llvm.git

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@183625 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 49ef1e75f1c..3fdb5f0e583 100644
--- a/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -343,8 +343,9 @@ bool DAE::RemoveDeadArgumentsFromCallers(Function &Fn)
   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 &F) {
   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.
diff --git a/test/Transforms/DeadArgElim/variadic_safety.ll b/test/Transforms/DeadArgElim/variadic_safety.ll
new file mode 100644
index 00000000000..15f57bcfdcb
--- /dev/null
+++ b/test/Transforms/DeadArgElim/variadic_safety.ll
@@ -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)
+}