[llvm] r203513 - Module: Don't rename in getOrInsertFunction()

[Duncan P. N. Exon Smith]

Author: dexonsmith
Date: Mon Mar 10 18:42:28 2014
New Revision: 203513

URL: http://llvm.org/viewvc/llvm-project?rev=203513&view=rev
Log:
Module: Don't rename in getOrInsertFunction()

During LTO, user-supplied definitions of C library functions often
exist.  -instcombine uses Module::getOrInsertFunction() to get a handle
on library functions (e.g., @puts, when optimizing @printf).

Previously, Module::getOrInsertFunction() would rename any matching
functions with local linkage, and create a new declaration.  In LTO,
this is the opposite of desired behaviour, as it skips by the
user-supplied version of the library function and creates a new
undefined reference which the linker often cannot resolve.

After some discussing with Rafael on the list, it looks like it's
undesired behaviour.  If a consumer actually *needs* this behaviour, we
should add new API with a more explicit name.

I added two testcases: one specifically for the -instcombine behaviour
and one for the LTO flow.

<rdar://problem/16165191>

Added:
    llvm/trunk/test/LTO/keep-used-puts-during-instcombine.ll
    llvm/trunk/test/LTO/no-undefined-puts-when-implemented.ll
Modified:
    llvm/trunk/lib/IR/Module.cpp

Modified: llvm/trunk/lib/IR/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=203513&r1=203512&r2=203513&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Module.cpp (original)
+++ llvm/trunk/lib/IR/Module.cpp Mon Mar 10 18:42:28 2014
@@ -104,16 +104,6 @@ Constant *Module::getOrInsertFunction(St
     return New;                    // Return the new prototype.
   }
 
-  // Okay, the function exists.  Does it have externally visible linkage?
-  if (F->hasLocalLinkage()) {
-    // Clear the function's name.
-    F->setName("");
-    // Retry, now there won't be a conflict.
-    Constant *NewF = getOrInsertFunction(Name, Ty);
-    F->setName(Name);
-    return NewF;
-  }
-
   // If the function exists but has the wrong type, return a bitcast to the
   // right type.
   if (F->getType() != PointerType::getUnqual(Ty))

Added: llvm/trunk/test/LTO/keep-used-puts-during-instcombine.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/keep-used-puts-during-instcombine.ll?rev=203513&view=auto
==============================================================================
--- llvm/trunk/test/LTO/keep-used-puts-during-instcombine.ll (added)
+++ llvm/trunk/test/LTO/keep-used-puts-during-instcombine.ll Mon Mar 10 18:42:28 2014
@@ -0,0 +1,36 @@
+; RUN: opt -S -instcombine <%s | FileCheck %s
+; rdar://problem/16165191
+; llvm.compiler.used functions should not be renamed
+
+target triple = "x86_64-apple-darwin11"
+
+ at llvm.compiler.used = appending global [1 x i8*] [
+  i8* bitcast (i32(i8*)* @puts to i8*)
+  ], section "llvm.metadata"
+ at llvm.used = appending global [1 x i8*] [
+  i8* bitcast (i32(i32)* @uses_printf to i8*)
+  ], section "llvm.metadata"
+
+ at str = private unnamed_addr constant [13 x i8] c"hello world\0A\00"
+
+define i32 @uses_printf(i32 %i) {
+entry:
+  %s = getelementptr [13 x i8]* @str, i64 0, i64 0
+  call i32 (i8*, ...)* @printf(i8* %s)
+  ret i32 0
+}
+
+define internal hidden i32 @printf(i8* readonly nocapture %fmt, ...) {
+entry:
+  %ret = call i32 @bar(i8* %fmt)
+  ret i32 %ret
+}
+
+; CHECK: define {{.*}} @puts(
+define internal hidden i32 @puts(i8* %s) {
+entry:
+  %ret = call i32 @bar(i8* %s)
+  ret i32 %ret
+}
+
+declare i32 @bar(i8*)

Added: llvm/trunk/test/LTO/no-undefined-puts-when-implemented.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/no-undefined-puts-when-implemented.ll?rev=203513&view=auto
==============================================================================
--- llvm/trunk/test/LTO/no-undefined-puts-when-implemented.ll (added)
+++ llvm/trunk/test/LTO/no-undefined-puts-when-implemented.ll Mon Mar 10 18:42:28 2014
@@ -0,0 +1,40 @@
+; RUN: llvm-as <%s >%t1
+; RUN: llvm-lto -exported-symbol=_uses_puts -exported-symbol=_uses_printf -o - %t1 | \
+; RUN: llvm-nm | \
+; RUN: FileCheck %s
+; rdar://problem/16165191
+; runtime library implementations should not be renamed
+
+target triple = "x86_64-apple-darwin11"
+
+ at str = private unnamed_addr constant [13 x i8] c"hello world\0A\00"
+
+; CHECK-NOT: U _puts
+; CHECK: T _uses_printf
+; CHECK: T _uses_puts
+define i32 @uses_puts(i32 %i) {
+entry:
+  %s = call i8* @foo(i32 %i)
+  %ret = call i32 @puts(i8* %s)
+  ret i32 %ret
+}
+define i32 @uses_printf(i32 %i) {
+entry:
+  %s = getelementptr [13 x i8]* @str, i64 0, i64 0
+  call i32 (i8*, ...)* @printf(i8* %s)
+  ret i32 0
+}
+
+define hidden i32 @printf(i8* readonly nocapture %fmt, ...) {
+entry:
+  %ret = call i32 @bar(i8* %fmt)
+  ret i32 %ret
+}
+define hidden i32 @puts(i8* %s) {
+entry:
+  %ret = call i32 @bar(i8* %s)
+  ret i32 %ret
+}
+
+declare i8* @foo(i32)
+declare i32 @bar(i8*)