From d84b17e157bb27bed236a400cccf4562d0b19d96 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Sun, 9 Jun 2013 02:17:27 +0000 Subject: [PATCH] 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 --- .../IPO/DeadArgumentElimination.cpp | 22 ++++++++--- .../Transforms/DeadArgElim/variadic_safety.ll | 38 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 test/Transforms/DeadArgElim/variadic_safety.ll 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) +}