[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv abandoned this revision.
george.burgess.iv added a comment.

Nothing in the real world :)

I was writing test-cases for a soon-to-be-in-your-inbox patch, and initially 
wrote `memcpy` recursion in terms of `memcpy`, rather than `__builtin_memcpy`. 
Wasn't sure if this was an oversight, or intended. The comments around the use 
of this through `isTriviallyRecursive` said "a[n externally-available] function 
that calls itself is clearly not equivalent to the real implementation." Since 
this behavior is intentional, I'll see if I can make that commentary a bit more 
clear instead.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78148/new/

https://reviews.llvm.org/D78148



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Could you go into a little more detail what problem you're trying to resolve?

isTriviallyRecursive is specifically a narrow hack to handle weird cases where 
a function is trying to hide the fact that it's calling itself, in ways that 
would convince gcc that the called function is different.  If a function is 
actually explicitly calling itself, it's on the user to make sure they don't 
write an infinite loop, I think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78148/new/

https://reviews.llvm.org/D78148



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

[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 function was not catching all forms of trivial recursion, meaning:

  void *memcpy(void *a, const void *b, size_t n) {
return __builtin_memcpy(a, b, n);
  }

would be considered trivially recursive, whereas

  void *memcpy(void *a, const void *b, size_t n) {
return memcpy(a, b, n);
  }

would not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78148

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-no-emission-trivial-recursion.c


Index: clang/test/CodeGen/memcpy-no-emission-trivial-recursion.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-no-emission-trivial-recursion.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+//
+// Verifies that clang doesn't emit trivially-recursive extern inline 
functions.
+
+typedef unsigned long size_t;
+
+// always_inline is used so clang will emit any available_externally function 
in
+// -O0.
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
+__attribute__((gnu_inline))
+
+void some_other_fn(void);
+
+AVAILABLE_EXTERNALLY void *some_fn() {
+  some_other_fn();
+  return some_fn();
+}
+
+// CHECK-LABEL: define void @foo
+void foo() {
+  // CHECK-NOT: call void @some_other_fn
+  some_fn();
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2651,15 +2651,19 @@
 namespace {
   struct FunctionIsDirectlyRecursive
   : public ConstStmtVisitor {
+const FunctionDecl *OriginalFD;
 const StringRef Name;
 const Builtin::Context 
-FunctionIsDirectlyRecursive(StringRef N, const Builtin::Context )
-: Name(N), BI(C) {}
+FunctionIsDirectlyRecursive(const FunctionDecl *OriginalFD, StringRef N,
+const Builtin::Context )
+: OriginalFD(OriginalFD), Name(N), BI(C) {}
 
 bool VisitCallExpr(const CallExpr *E) {
   const FunctionDecl *FD = E->getDirectCallee();
   if (!FD)
 return false;
+  if (FD == OriginalFD)
+return true;
   AsmLabelAttr *Attr = FD->getAttr();
   if (Attr && Name == Attr->getLabel())
 return true;
@@ -2762,7 +2766,7 @@
 Name = FD->getName();
   }
 
-  FunctionIsDirectlyRecursive Walker(Name, Context.BuiltinInfo);
+  FunctionIsDirectlyRecursive Walker(FD, Name, Context.BuiltinInfo);
   const Stmt *Body = FD->getBody();
   return Body ? Walker.Visit(Body) : false;
 }


Index: clang/test/CodeGen/memcpy-no-emission-trivial-recursion.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-no-emission-trivial-recursion.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+//
+// Verifies that clang doesn't emit trivially-recursive extern inline functions.
+
+typedef unsigned long size_t;
+
+// always_inline is used so clang will emit any available_externally function in
+// -O0.
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
+__attribute__((gnu_inline))
+
+void some_other_fn(void);
+
+AVAILABLE_EXTERNALLY void *some_fn() {
+  some_other_fn();
+  return some_fn();
+}
+
+// CHECK-LABEL: define void @foo
+void foo() {
+  // CHECK-NOT: call void @some_other_fn
+  some_fn();
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2651,15 +2651,19 @@
 namespace {
   struct FunctionIsDirectlyRecursive
   : public ConstStmtVisitor {
+const FunctionDecl *OriginalFD;
 const StringRef Name;
 const Builtin::Context 
-FunctionIsDirectlyRecursive(StringRef N, const Builtin::Context )
-: Name(N), BI(C) {}
+FunctionIsDirectlyRecursive(const FunctionDecl *OriginalFD, StringRef N,
+const Builtin::Context )
+: OriginalFD(OriginalFD), Name(N), BI(C) {}
 
 bool VisitCallExpr(const CallExpr *E) {
   const FunctionDecl *FD = E->getDirectCallee();
   if (!FD)
 return false;
+  if (FD == OriginalFD)
+return true;
   AsmLabelAttr *Attr = FD->getAttr();
   if (Attr && Name == Attr->getLabel())
 return true;
@@ -2762,7 +2766,7 @@
 Name = FD->getName();
   }
 
-  FunctionIsDirectlyRecursive Walker(Name, Context.BuiltinInfo);
+  FunctionIsDirectlyRecursive Walker(FD, Name, Context.BuiltinInfo);
   const Stmt *Body = FD->getBody();
   return Body ? Walker.Visit(Body) : false;
 }
___
cfe-commits mailing