Index: clang/lib/CodeGen/CGCall.cpp =================================================================== --- clang/lib/CodeGen/CGCall.cpp +++ clang/lib/CodeGen/CGCall.cpp @@ -4975,6 +4975,32 @@ Callee.getAbstractInfo(), Attrs, CallingConv, /*AttrOnCallSite=*/true); + // Calling a function that aliases to self through an AsmLabel is a pattern + // used by glibc for fortified functions. Mark the call as Builtin and the + // called function as NoBuiltin. + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) { + bool MarkBuiltin = false; + if (AsmLabelAttr *Attr = FD->getAttr()) { + MarkBuiltin |= CurFn->getName() == Attr->getLabel() && + Builtin::Context::isBuiltinFunc(Attr->getLabel()); + } + unsigned BuiltinID = FD->getBuiltinID(); + Builtin::Context &BI = getContext().BuiltinInfo; + if (BuiltinID && BI.isLibFunction(BuiltinID)) { + StringRef BuiltinName = BI.getName(BuiltinID); + MarkBuiltin |= BuiltinName.startswith("__builtin_") && + CurFn->getName() == BuiltinName.slice(strlen("__builtin_"), + StringRef::npos); + } + if (MarkBuiltin) { + Attrs = Attrs.addAttribute(getLLVMContext(), + llvm::AttributeList::FunctionIndex, + llvm::Attribute::Builtin); + CurFn->addAttribute(llvm::AttributeList::FunctionIndex, + llvm::Attribute::NoBuiltin); + } + } + if (const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl)) if (FD->hasAttr()) // All calls within a strictfp function are marked strictfp Index: clang/lib/CodeGen/CodeGenModule.h =================================================================== --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -506,7 +506,6 @@ void createOpenMPRuntime(); void createCUDARuntime(); - bool isTriviallyRecursive(const FunctionDecl *F); bool shouldEmitFunction(GlobalDecl GD); bool shouldOpportunisticallyEmitVTables(); /// Map used to be sure we don't emit the same CompoundLiteral twice. Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -2869,38 +2869,6 @@ } namespace { - struct FunctionIsDirectlyRecursive - : public ConstStmtVisitor { - const StringRef Name; - const Builtin::Context &BI; - FunctionIsDirectlyRecursive(StringRef N, const Builtin::Context &C) - : Name(N), BI(C) {} - - bool VisitCallExpr(const CallExpr *E) { - const FunctionDecl *FD = E->getDirectCallee(); - if (!FD) - return false; - AsmLabelAttr *Attr = FD->getAttr(); - if (Attr && Name == Attr->getLabel()) - return true; - unsigned BuiltinID = FD->getBuiltinID(); - if (!BuiltinID || !BI.isLibFunction(BuiltinID)) - return false; - StringRef BuiltinName = BI.getName(BuiltinID); - if (BuiltinName.startswith("__builtin_") && - Name == BuiltinName.slice(strlen("__builtin_"), StringRef::npos)) { - return true; - } - return false; - } - - bool VisitStmt(const Stmt *S) { - for (const Stmt *Child : S->children()) - if (Child && this->Visit(Child)) - return true; - return false; - } - }; // Make sure we're not referencing non-imported vars or functions. struct DLLImportFunctionVisitor @@ -2966,26 +2934,6 @@ }; } -// isTriviallyRecursive - Check if this function calls another -// decl that, because of the asm attribute or the other decl being a builtin, -// ends up pointing to itself. -bool -CodeGenModule::isTriviallyRecursive(const FunctionDecl *FD) { - StringRef Name; - if (getCXXABI().getMangleContext().shouldMangleDeclName(FD)) { - // asm labels are a special kind of mangling we have to support. - AsmLabelAttr *Attr = FD->getAttr(); - if (!Attr) - return false; - Name = Attr->getLabel(); - } else { - Name = FD->getName(); - } - - FunctionIsDirectlyRecursive Walker(Name, Context.BuiltinInfo); - const Stmt *Body = FD->getBody(); - return Body ? Walker.Visit(Body) : false; -} bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) { if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage) @@ -3014,12 +2962,7 @@ } } - // PR9614. Avoid cases where the source code is lying to us. An available - // externally function should have an equivalent function somewhere else, - // but a function that calls itself through asm label/`__builtin_` trickery is - // clearly not equivalent to the real implementation. - // This happens in glibc's btowc and in some configure checks. - return !isTriviallyRecursive(F); + return true; } bool CodeGenModule::shouldOpportunisticallyEmitVTables() { Index: clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c =================================================================== --- clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c +++ clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c @@ -1,15 +1,26 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -disable-llvm-passes -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NO-OPT %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-OPT %s // // Verifies that clang doesn't mark an inline builtin definition as `nobuiltin` // if the builtin isn't emittable. typedef unsigned long size_t; -// always_inline is used so clang will emit this body. Otherwise, we need >= -// -O1. +// always_inline is used so clang will emit this body. #define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \ __attribute__((gnu_inline)) +// When always_inliner is on, @memcpy is removed and inlined +// CHECK-OPT-NOT: define i8* @memcpy +// CHECK-OPT-LABEL: define void @foo +// CHECK-OPT: call void @llvm.memcpy + +// When always_inliner is off, @memcpy is generated and called +// CHECK-NO-OPT-LABEL: define void @foo +// CHECK-NO-OPT: call i8* @memcpy +// CHECK-NO-OPT: define available_externally i8* @memcpy + +// This is inlined and suppressed at -O0 thanks to gnu_inline + always_inline AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) { return __builtin_memcpy(a, b, c); } @@ -17,9 +28,7 @@ // CHECK-LABEL: define{{.*}} void @foo void foo(void *a, const void *b, size_t c) { // Clang will always _emit_ this as memcpy. LLVM turns it into @llvm.memcpy - // later on if optimizations are enabled. - // CHECK: call i8* @memcpy + // even at -O0 memcpy(a, b, c); } -// CHECK-NOT: nobuiltin Index: clang/test/CodeGen/memmove-always-inline-definition-used.c =================================================================== --- /dev/null +++ clang/test/CodeGen/memmove-always-inline-definition-used.c @@ -0,0 +1,42 @@ +// Verifies that even at -O0, the inline definition of memmove has precedence over the builtin +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -disable-llvm-passes -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NO-INLINED %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-INLINED %s + +#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \ +__attribute__((gnu_inline)) + +typedef unsigned long size_t; + +extern void *memmove_alias(void *a, const void *b, size_t c) __asm__("memmove"); + +// Under -disable-llvm-passes foo is calling the actual memmove function and not the builtin. +// The call to memmove through memmove_alias is marked as builtin to avoid the recursive call. +// CHECK-NO-INLINED-LABEL: define void @foo +// CHECK-NO-INLINED: call i8* @memmove +// +// CHECK-NO-INLINED: define available_externally i8* @memmove(i8* %{{[a-z0-9]*}}, i8* %{{[a-z0-9]*}}, i64 %{{[a-z0-9]*}}) #[[ATTR1:[0-9]+]] +// CHECK-NO-INLINED: call void @llvm.memcpy +// CHECK-NO-INLINED: call i8* @memmove({{.*}}) #[[ATTR2:[0-9]+]] +// CHECK-NO-INLINED: attributes #[[ATTR1]] = { alwaysinline nobuiltin +// CHECK-NO-INLINED: attributes #[[ATTR2]] = { builtin } + +// Without -disable-llvm-passes always_inline triggers, the call to memmove in foo is inlined. +// CHECK-INLINED-NOT: define available_externally i8* @memmove +// CHECK-INLINED-LABEL: define void @foo +// CHECK-INLINED: call void @llvm.memcpy +// CHECK-INLINED: call i8* @memmove({{.*}}) #[[ATTR:[0-9]+]] +// CHECK-INLINED: declare i8* @memmove +// CHECK-INLINED: attributes #[[ATTR]] = { builtin + +AVAILABLE_EXTERNALLY void *memmove(void *a, const void *b, size_t c) { + if (c == 1 && a != b) { + return __builtin_memcpy(a, b, c); + } else { + return memmove_alias(a, b, c); + } +} + +void foo(void *a, const void *b, size_t c) { + memmove(a, b, c); +} + Index: clang/test/CodeGen/pr9614.c =================================================================== --- clang/test/CodeGen/pr9614.c +++ clang/test/CodeGen/pr9614.c @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - | FileCheck --check-prefix CHECK-INLINED %s extern void foo_alias (void) __asm ("foo"); inline void foo (void) { @@ -30,16 +31,58 @@ memchr("", '.', 0); } +// With -disable-llvm-passes, the always_inliner doesn't run so we can observe +// how clang handles always inline + gnu inline redefinition of builtins. +// each builtin (re)definition calls the actual builtin using either the +// intrinsic or a call to selfe flagged as 'builtin', while the function itself +// is flagged as 'nobuiltin' + // CHECK-LABEL: define{{.*}} void @f() // CHECK: call void @foo() // CHECK: call i32 @abs(i32 0) // CHECK: call i8* @strrchr( -// CHECK: call void @llvm.prefetch.p0i8( +// CHECK: call void @prefetch( // CHECK: call i8* @memchr( // CHECK: ret void +// +// CHECK-LABEL: define available_externally i32 @abs({{.*}}) #2 +// CHECK: call i32 @abs({{.*}}) #7 +// +// CHECK-LABEL: define available_externally i8* @strrchr({{.*}}) #3 +// CHECK: call i8* @strrchr({{.*}}) #8 +// +// CHECK-LABEL: define available_externally void @prefetch() #4 +// CHECK: call void @llvm.prefetch.p0i8 +// +// CHECK-LABEL: define available_externally i8* @memchr({{.*}}) #3 +// CHECK: call i8* @memchr({{.*}}) #8 +// +// CHECK: attributes #2 = { alwaysinline nobuiltin nounwind +// CHECK: attributes #3 = { alwaysinline nobuiltin nounwind +// CHECK: attributes #4 = { alwaysinline nounwind +// CHECK: attributes #7 = { builtin } +// CHECK: attributes #8 = { builtin nounwind } + +// With always_inliner one, the inlined definition all gets removed +// how clang handles always inline + gnu inline redefinition of builtins. +// each builtin (re)definition calls the actual builtin using either the +// intrinsic or a call to selfe flagged as 'builtin', while the function itself +// is flagged as 'nobuiltin' -// CHECK: declare void @foo() -// CHECK: declare i32 @abs(i32 -// CHECK: declare i8* @strrchr(i8*, i32) -// CHECK: declare i8* @memchr( -// CHECK: declare void @llvm.prefetch.p0i8( +// CHECK-INLINED-NOT: define void @foo +// CHECK-INLINED-NOT: define i32 @abs +// CHECK-INLINED-NOT: define i8* @strrchr +// CHECK-INLINED-NOT: define i8* @memchr +// CHECK-INLINED-NOT: define void @llvm.prefetch.p0i8 +// +// CHECK-INLINED: define void @f() +// CHECK-INLINED: call void @foo +// CHECK-INLINED: call i8* @strrchr +// CHECK-INLINED: call void @llvm.prefetch.p0i8 +// CHECK-INLINED: call i8* @memchr +// +// CHECK-INLINED: declare void @foo +// CHECK-INLINED: declare i32 @abs +// CHECK-INLINED: declare i8* @strrchr +// CHECK-INLINED: declare i8* @memchr +// CHECK-INLINED: declare void @llvm.prefetch.p0i8 Index: llvm/lib/Analysis/InlineCost.cpp =================================================================== --- llvm/lib/Analysis/InlineCost.cpp +++ llvm/lib/Analysis/InlineCost.cpp @@ -2634,9 +2634,9 @@ if (!Call) continue; - // Disallow recursive calls. + // Disallow recursive calls, unless marked as builtin Function *Callee = Call->getCalledFunction(); - if (&F == Callee) + if (!Call->hasFnAttr(Attribute::Builtin) && &F == Callee) return InlineResult::failure("recursive call"); // Disallow calls which expose returns-twice to a function not previously Index: llvm/lib/IR/Function.cpp =================================================================== --- llvm/lib/IR/Function.cpp +++ llvm/lib/IR/Function.cpp @@ -1612,9 +1612,10 @@ return false; // Check if the function is used by anything other than a blockaddress. - for (const User *U : users()) + for (const User *U : users()) { if (!isa(U)) return false; + } return true; } Index: llvm/lib/Transforms/IPO/Inliner.cpp =================================================================== --- llvm/lib/Transforms/IPO/Inliner.cpp +++ llvm/lib/Transforms/IPO/Inliner.cpp @@ -57,6 +57,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Transforms/Utils/BuildLibCalls.h" #include "llvm/Transforms/Utils/CallPromotionUtils.h" #include "llvm/Transforms/Utils/Cloning.h" #include "llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h" @@ -602,6 +603,31 @@ // them. F->removeDeadConstantUsers(); + // If all remaining users are calling F and are marked as Builtin while F is + // NoBuiltin, then we can just promote F to the equivalent builtin. This + // situation happens after inlining always_inline + gnu_inline redefinition + // of builtins as found in glibc headers. + if (F->hasFnAttribute(Attribute::NoBuiltin) && + F->hasFnAttribute(Attribute::AlwaysInline)) { + bool AllUsesAreBuiltin = true; + for (User *U : F->users()) { + if (auto const *CB = dyn_cast(U)) { + if (CB->getCalledFunction() == F && + CB->hasFnAttr(Attribute::Builtin)) { + continue; + } + } + AllUsesAreBuiltin = false; + break; + } + if (AllUsesAreBuiltin) { + F->deleteBody(); + F->setAttributes({}); + inferLibFuncAttributes( + *F, getAnalysis().getTLI(*F)); + } + } + if (!F->isDefTriviallyDead()) continue;