[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added reviewers: efriedma, serge-sans-paille.
george.burgess.iv added a project: clang.
Herald added a subscriber: cfe-commits.

This suboptimality was encountered in Linux (see some discussion on D71082 <https://reviews.llvm.org/D71082>, and https://github.com/ClangBuiltLinux/linux/issues/979).

While it'd be great to have the kernel's FORTIFY working well with clang, it seems best to preserve old functionality in cases where D71082 <https://reviews.llvm.org/D71082> can't emit a FORTIFY'ed wrapper of a builtin. With this patch, the `memcpy` in the test-case here https://reviews.llvm.org/D71082#1953975 compiles back to a `mov`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78162

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c


Index: clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %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.
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
+    __attribute__((gnu_inline))
+
+AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) {
+  return __builtin_memcpy(a, b, c);
+}
+
+// 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
+  memcpy(a, b, c);
+}
+
+// CHECK-NOT: nobuiltin
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1908,7 +1908,8 @@
   else if (const auto *SA = FD->getAttr<SectionAttr>())
      F->setSection(SA->getName());
 
-  if (FD->isInlineBuiltinDeclaration()) {
+  // If we plan on emitting this inline builtin, we can't treat it as a builtin.
+  if (FD->isInlineBuiltinDeclaration() && shouldEmitFunction(FD)) {
     F->addAttribute(llvm::AttributeList::FunctionIndex,
                     llvm::Attribute::NoBuiltin);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78162.257504.patch
Type: text/x-patch
Size: 1676 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200414/6ecab9a4/attachment.bin>