[PATCH] D56865: [MergeFunc] Allow merging identical vararg functions using aliases

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: nikic, whitequark, aschwaighofer, eeckstein.
Herald added a subscriber: hiraditya.

Thanks to Nikita Popov for pointing out this missed case.

This is a follow-up to r351411, which disabled function merging for
vararg functions outright due to a miscompile (see llvm.org/PR40345).


https://reviews.llvm.org/D56865

Files:
  llvm/lib/Transforms/IPO/MergeFunctions.cpp
  llvm/test/Transforms/MergeFunc/va_arg.ll


Index: llvm/test/Transforms/MergeFunc/va_arg.ll
===================================================================
--- llvm/test/Transforms/MergeFunc/va_arg.ll
+++ llvm/test/Transforms/MergeFunc/va_arg.ll
@@ -1,14 +1,18 @@
 ; RUN: opt -S -mergefunc < %s | FileCheck %s
+; RUN: opt -S -mergefunc -mergefunc-use-aliases < %s | FileCheck %s -check-prefix=ALIAS
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
+; ALIAS: @_Z9simple_vaPKcz = unnamed_addr alias void (i8*, ...), void (i8*, ...)* @_Z10simple_va2PKcz
+; ALIAS-NOT: @_Z9simple_vaPKcz
+
 %struct.__va_list_tag = type { i32, i32, i8*, i8* }
 
 ; CHECK-LABEL: define {{.*}}@_Z9simple_vaPKcz
 ; CHECK: call void @llvm.va_start
 ; CHECK: call void @llvm.va_end
-define dso_local void @_Z9simple_vaPKcz(i8* nocapture readnone, ...) local_unnamed_addr {
+define dso_local void @_Z9simple_vaPKcz(i8* nocapture readnone, ...) unnamed_addr {
   %2 = alloca [1 x %struct.__va_list_tag], align 16
   %3 = bitcast [1 x %struct.__va_list_tag]* %2 to i8*
   call void @llvm.va_start(i8* nonnull %3)
@@ -54,7 +58,7 @@
 ; CHECK-LABEL: define {{.*}}@_Z10simple_va2PKcz
 ; CHECK: call void @llvm.va_start
 ; CHECK: call void @llvm.va_end
-define dso_local void @_Z10simple_va2PKcz(i8* nocapture readnone, ...) local_unnamed_addr {
+define dso_local void @_Z10simple_va2PKcz(i8* nocapture readnone, ...) unnamed_addr {
   %2 = alloca [1 x %struct.__va_list_tag], align 16
   %3 = bitcast [1 x %struct.__va_list_tag]* %2 to i8*
   call void @llvm.va_start(i8* nonnull %3)
Index: llvm/lib/Transforms/IPO/MergeFunctions.cpp
===================================================================
--- llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -281,8 +281,8 @@
   // Replace G with an alias to F (deleting function G)
   void writeAlias(Function *F, Function *G);
 
-  // Replace G with an alias to F if possible, or a thunk to F if
-  // profitable. Returns false if neither is the case.
+  // Replace G with an alias to F if possible, or a thunk to F if possible.
+  // Returns false if neither is the case.
   bool writeThunkOrAlias(Function *F, Function *G);
 
   /// Replace function F with function G in the function tree.
@@ -385,8 +385,7 @@
 
 /// Check whether \p F is eligible for function merging.
 static bool isEligibleForMerging(Function &F) {
-  return !F.isDeclaration() && !F.hasAvailableExternallyLinkage() &&
-         !F.isVarArg();
+  return !F.isDeclaration() && !F.hasAvailableExternallyLinkage();
 }
 
 bool MergeFunctions::runOnModule(Module &M) {
@@ -660,12 +659,16 @@
   LLVM_DEBUG(dbgs() << " }\n");
 }
 
-// Don't merge tiny functions using a thunk, since it can just end up
-// making the function larger.
-static bool isThunkProfitable(Function * F) {
+/// Whether this function may be replaced by a forwarding thunk.
+static bool canCreateThunkFor(Function *F) {
+  if (F->isVarArg())
+    return false;
+
+  // Don't merge tiny functions using a thunk, since it can just end up
+  // making the function larger.
   if (F->size() == 1) {
     if (F->front().size() <= 2) {
-      LLVM_DEBUG(dbgs() << "isThunkProfitable: " << F->getName()
+      LLVM_DEBUG(dbgs() << "canCreateThunkFor: " << F->getName()
                         << " is too small to bother creating a thunk for\n");
       return false;
     }
@@ -793,7 +796,7 @@
     writeAlias(F, G);
     return true;
   }
-  if (isThunkProfitable(F)) {
+  if (canCreateThunkFor(F)) {
     writeThunk(F, G);
     return true;
   }
@@ -808,9 +811,9 @@
     // Both writeThunkOrAlias() calls below must succeed, either because we can
     // create aliases for G and NewF, or because a thunk for F is profitable.
     // F here has the same signature as NewF below, so that's what we check.
-    if (!isThunkProfitable(F) && (!canCreateAliasFor(F) || !canCreateAliasFor(G))) {
+    if (!canCreateThunkFor(F) &&
+        (!canCreateAliasFor(F) || !canCreateAliasFor(G)))
       return;
-    }
 
     // Make them both thunks to the same internal function.
     Function *NewF = Function::Create(F->getFunctionType(), F->getLinkage(),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56865.182332.patch
Type: text/x-patch
Size: 4159 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190117/39eab64a/attachment.bin>