From f5d17528ee24cfd3f77a4b11b82486a5a9a6d920 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 10 Mar 2014 23:42:28 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@203513 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/IR/Module.cpp | 10 ----- test/LTO/keep-used-puts-during-instcombine.ll | 36 +++++++++++++++++ .../LTO/no-undefined-puts-when-implemented.ll | 40 +++++++++++++++++++ 3 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 test/LTO/keep-used-puts-during-instcombine.ll create mode 100644 test/LTO/no-undefined-puts-when-implemented.ll diff --git a/lib/IR/Module.cpp b/lib/IR/Module.cpp index c8c07f27a00..1accd471a9a 100644 --- a/lib/IR/Module.cpp +++ b/lib/IR/Module.cpp @@ -104,16 +104,6 @@ Constant *Module::getOrInsertFunction(StringRef Name, 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)) diff --git a/test/LTO/keep-used-puts-during-instcombine.ll b/test/LTO/keep-used-puts-during-instcombine.ll new file mode 100644 index 00000000000..1dc63dd4fcd --- /dev/null +++ b/test/LTO/keep-used-puts-during-instcombine.ll @@ -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" + +@llvm.compiler.used = appending global [1 x i8*] [ + i8* bitcast (i32(i8*)* @puts to i8*) + ], section "llvm.metadata" +@llvm.used = appending global [1 x i8*] [ + i8* bitcast (i32(i32)* @uses_printf to i8*) + ], section "llvm.metadata" + +@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*) diff --git a/test/LTO/no-undefined-puts-when-implemented.ll b/test/LTO/no-undefined-puts-when-implemented.ll new file mode 100644 index 00000000000..18f5d214812 --- /dev/null +++ b/test/LTO/no-undefined-puts-when-implemented.ll @@ -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" + +@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*)