[PATCH] D60088: [GlobalOpt][SampleFDO] Add an option to control whether to rename alias target

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: davidxl, danielcdh.
Herald added a project: LLVM.

GlobalOpt could rename an alias target (aliasee) to its alias when the alias is the only one targeting the aliasee. However, this could cause problem for SampleFDO.  Because once the alias target is renamed and then removed, it will not appear in the final binary and the profile. During profile use, sample profile loader will not find the function profile at the time of profile annotation.

We need to keep the alias target even for non-SampleFDO build targets if only it is possible that sample profile may be used in their next build cycle, so we add an option to make disabling alias target renaming possible.

In the long run, we may want to look at how to make sample profile loader not only look for profile with the target function name, but also those profiles with the names of all possible aliases. That may bring the benefit of reducing the sample profile size in addition.


Repository:
  rL LLVM

https://reviews.llvm.org/D60088

Files:
  lib/Transforms/IPO/GlobalOpt.cpp
  test/Transforms/GlobalOpt/aliase-target-rename.ll


Index: test/Transforms/GlobalOpt/aliase-target-rename.ll
===================================================================
--- test/Transforms/GlobalOpt/aliase-target-rename.ll
+++ test/Transforms/GlobalOpt/aliase-target-rename.ll
@@ -0,0 +1,25 @@
+; RUN: opt < %s -globalopt -enable-alias-target-renaming=true -S | FileCheck %s
+; RUN: opt < %s -globalopt -enable-alias-target-renaming=false -S | FileCheck %s --check-prefix=NORENAME
+; RUN: opt < %s -passes=globalopt -enable-alias-target-renaming=true -S | FileCheck %s
+; RUN: opt < %s -passes=globalopt -enable-alias-target-renaming=false -S | FileCheck %s --check-prefix=NORENAME
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at str = private unnamed_addr constant [13 x i8] c"In weak1.c:f\00", align 1
+
+; CHECK-NOT: @strongalias = hidden alias void (), void ()* @aliasee
+; NORENAME: @strongalias = hidden alias void (), void ()* @aliasee
+ at strongalias = hidden alias void (), void ()* @aliasee
+
+declare i32 @puts(i8* nocapture readonly)
+
+; CHECK: define hidden void @strongalias()
+; NORENAME: define internal void @aliasee()
+define internal void @aliasee() #0 {
+entry:
+  %puts = tail call i32 @puts(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @str, i64 0, i64 0))
+  ret void
+}
+
+
Index: lib/Transforms/IPO/GlobalOpt.cpp
===================================================================
--- lib/Transforms/IPO/GlobalOpt.cpp
+++ lib/Transforms/IPO/GlobalOpt.cpp
@@ -98,6 +98,11 @@
                                     "calling conv to all internal functions."),
                            cl::init(false), cl::Hidden);
 
+static cl::opt<bool> EnableAliasTargetRenaming(
+    "enable-alias-target-renaming",
+    cl::desc("Enable renaming alias target to the alias"), cl::init(true),
+    cl::Hidden);
+
 static cl::opt<int> ColdCCRelFreq(
     "coldcc-rel-freq", cl::Hidden, cl::init(2), cl::ZeroOrMore,
     cl::desc(
@@ -2692,16 +2697,17 @@
   return U.usedCount(&GA) || U.compilerUsedCount(&GA);
 }
 
-static bool hasUsesToReplace(GlobalAlias &GA, const LLVMUsed &U,
-                             bool &RenameTarget) {
-  RenameTarget = false;
-  bool Ret = false;
-  if (hasUseOtherThanLLVMUsed(GA, U))
-    Ret = true;
+static bool hasUsesToReplace(GlobalAlias &GA, const LLVMUsed &U) {
+  return hasUseOtherThanLLVMUsed(GA, U);
+}
+
+static bool wantToRenameTarget(GlobalAlias &GA, const LLVMUsed &U) {
+  if (!EnableAliasTargetRenaming)
+    return false;
 
   // If the alias is externally visible, we may still be able to simplify it.
   if (!mayHaveOtherReferences(GA, U))
-    return Ret;
+    return false;
 
   // If the aliasee has internal linkage, give it the name and linkage
   // of the alias, and delete the alias.  This turns:
@@ -2712,15 +2718,14 @@
   Constant *Aliasee = GA.getAliasee();
   GlobalValue *Target = cast<GlobalValue>(Aliasee->stripPointerCasts());
   if (!Target->hasLocalLinkage())
-    return Ret;
+    return false;
 
   // Do not perform the transform if multiple aliases potentially target the
   // aliasee. This check also ensures that it is safe to replace the section
   // and other attributes of the aliasee with those of the alias.
   if (hasMoreThanOneUseOtherThanLLVMUsed(*Target, U))
-    return Ret;
+    return false;
 
-  RenameTarget = true;
   return true;
 }
 
@@ -2760,8 +2765,8 @@
     Target->removeDeadConstantUsers();
 
     // Make all users of the alias use the aliasee instead.
-    bool RenameTarget;
-    if (!hasUsesToReplace(*J, Used, RenameTarget))
+    bool RenameTarget = wantToRenameTarget(*J, Used);
+    if (!hasUsesToReplace(*J, Used) && !RenameTarget)
       continue;
 
     J->replaceAllUsesWith(ConstantExpr::getBitCast(Aliasee, J->getType()));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60088.193134.patch
Type: text/x-patch
Size: 3769 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190401/a3e025bc/attachment.bin>