[patch] Teach GlobalDCE how to remove empty global_ctor entries
Richard Smith
richard at metafoo.co.uk
Thu May 1 23:23:25 PDT 2014
On Thu, May 1, 2014 at 9:29 PM, Nico Weber <thakis at chromium.org> wrote:
> On Thu, May 1, 2014 at 8:31 PM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
> > OptimizeGlobalCtorsList
> >
> > start the name with a lowercase.
>
> Done. (I'm just moving them around, but since that modifies "svn
> blame" output already, renaming at the same time is a good idea.)
>
> >
> > +bool IsEmptyFunction(Function* F) {
> > + BasicBlock &Entry = F->getEntryBlock();
> > + return Entry.size() == 1 && isa<ReturnInst>(Entry.front());
> > }
> >
> > There is no way to cast to void in llvm, right? The thing I am afraid
> > of is if a trapping constant can be hidden in there.
>
> global_ctors are always arrray of { i32, void ()* }…oooh I see what
> you're saying. I think it should be safe as-is, but I added a check
> for RI.getReturnValue() too.
>
> > +/// Given a specified llvm.global_ctors list, install the
> > +/// specified array, returning the new global to use.
> > +void InstallGlobalCtors(GlobalVariable *GCL,
> >
> > lowercase. Returning? install?
>
> Done.
>
> > ParseGlobalCtors
> >
> > lowercase.
>
> Done.
>
> > + if (!GV->hasUniqueInitializer()) return nullptr;
> >
> > please clang-format.
>
> Done.
>
> > + // Init priority must be standard.
> >
> > Do you know why? At least for the empty case it should be valid to
> > drop constructors of any priority.
>
> I don't know. Constructors with init priority are rare, and empty
> constructors with init priority are probably rarer still though.
>
> > + ... Call "Remove" for
> >
> >
> > It is actually a predicate, so maybe ShouldRemove?
>
> Done. I also changed this to a function pointer with a context
> argument, since
>
> http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features
> says that std::function cannot be used yet.
It looks like you don't actually want std::function (you don't want an
owned and managed copy of a callable), you just want a non-owning,
type-erasing callable reference. I wanted one of those about a month ago,
and wrote a special-case handler for it at the time, but you've motivated
me to implement the general case.
Attached is a patch for an llvm::function_ref (a non-owning, type-erased
callable) and a sample use. This would be usable as a drop-in replacement
for std::function in your previous patch.
> > This is a case where an integration test might be a good idea. Do you
> > have a reasonably size test where this change causes a ctor to be
> > deleted at -O2? We might want to test that, so that even if we go with
> > a different optimization strategy in the future we still have a test
> > of the end result (the ctor gets deleted somewhere in -O2).
>
> Yes, there's one in the bug. I can check that in on the clang side
> once this patch is in.
>
> Thanks!
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140501/42052db4/attachment.html>
-------------- next part --------------
Index: include/llvm/ADT/STLExtras.h
===================================================================
--- include/llvm/ADT/STLExtras.h (revision 207731)
+++ include/llvm/ADT/STLExtras.h (working copy)
@@ -55,6 +55,126 @@
}
};
+/// An efficient, type-erasing, non-owning reference to a callable. This is
+/// intended for use as the type of a function parameter that is not used
+/// after the function in question returns.
+///
+/// This class does not own the callable, so it is not in general safe to store
+/// a function_ref.
+template<typename Fn> class function_ref;
+
+#if LLVM_HAS_VARIADIC_TEMPLATES
+
+template<typename Ret, typename ...Params>
+class function_ref<Ret(Params...)> {
+ Ret (*callback)(void *callable, Params ...params);
+ void *callable;
+
+ template<typename Callable>
+ static Ret callback_fn(void *callable, Params ...params) {
+ return reinterpret_cast<Callable&>(*callable)(
+ std::forward<Params>(params)...);
+ }
+
+public:
+ template<typename Callable>
+ function_ref(Callable &&callable)
+ : callback(callback_fn<Callable>), callable(&callable) {}
+ Ret operator()(Params ...params) const {
+ return callback(callable, std::forward<Params>(params)...);
+ }
+};
+
+#else
+
+template<typename Ret>
+class function_ref<Ret()> {
+ Ret (*callback)(void *callable);
+ void *callable;
+
+ template<typename Callable>
+ static Ret callback_fn(void *callable) {
+ return reinterpret_cast<Callable&>(*callable)();
+ }
+
+public:
+ template<typename Callable>
+ function_ref(Callable &&callable)
+ : callback(callback_fn<Callable>), callable(&callable) {}
+ Ret operator()() const { return callback(callable); }
+};
+
+template<typename Ret, typename Param1>
+class function_ref<Ret(Param1)> {
+ Ret (*callback)(void *callable, Param1 param1);
+ void *callable;
+
+ template<typename Callable>
+ static Ret callback_fn(void *callable, Param1 param1) {
+ return reinterpret_cast<Callable&>(*callable)(
+ std::forward<Param1>(param1));
+ }
+
+public:
+ template<typename Callable>
+ function_ref(Callable &&callable)
+ : callback(callback_fn<Callable>), callable(&callable) {}
+ Ret operator()(Param1 param1) {
+ return callback(callable, std::forward<Param1>(param1));
+ }
+};
+
+template<typename Ret, typename Param1, typename Param2>
+class function_ref<Ret(Param1, Param2)> {
+ Ret (*callback)(void *callable, Param1 param1, Param2 param2);
+ void *callable;
+
+ template<typename Callable>
+ static Ret callback_fn(void *callable, Param1 param1, Param2 param2) {
+ return reinterpret_cast<Callable&>(*callable)(
+ std::forward<Param1>(param1),
+ std::forward<Param3>(param2));
+ }
+
+public:
+ template<typename Callable>
+ function_ref(Callable &&callable)
+ : callback(callback_fn<Callable>), callable(&callable) {}
+ Ret operator()(Param1 param1, Param2 param2) {
+ return callback(callable,
+ std::forward<Param1>(param1),
+ std::forward<Param3>(param2));
+ }
+};
+
+template<typename Ret, typename Param1, typename Param2, typename Param3>
+class function_ref<Ret(Param1, Param2, Param3)> {
+ Ret (*callback)(void *callable, Param1 param1, Param2 param2, Param3 param3);
+ void *callable;
+
+ template<typename Callable>
+ static Ret callback_fn(void *callable, Param1 param1, Param2 param2,
+ Param3 param3) {
+ return reinterpret_cast<Callable&>(*callable)(
+ std::forward<Param1>(param1),
+ std::forward<Param2>(param2),
+ std::forward<Param3>(param3));
+ }
+
+public:
+ template<typename Callable>
+ function_ref(Callable &&callable)
+ : callback(callback_fn<Callable>), callable(&callable) {}
+ Ret operator()(Param1 param1, Param2 param2, Param3 param3) {
+ return callback(callable,
+ std::forward<Param1>(param1),
+ std::forward<Param2>(param2),
+ std::forward<Param3>(param3));
+ }
+};
+
+#endif
+
// deleter - Very very very simple method that is used to invoke operator
// delete on something. It is used like this:
//
Index: include/llvm/Support/CrashRecoveryContext.h
===================================================================
--- include/llvm/Support/CrashRecoveryContext.h (revision 207731)
+++ include/llvm/Support/CrashRecoveryContext.h (working copy)
@@ -12,11 +12,13 @@
#include <string>
+#include "llvm/ADT/STLExtras.h"
+
namespace llvm {
class StringRef;
class CrashRecoveryContextCleanup;
-
+
/// \brief Crash recovery helper object.
///
/// This class implements support for running operations in a safe context so
@@ -46,21 +48,10 @@
void *Impl;
CrashRecoveryContextCleanup *head;
- /// An adaptor to convert an arbitrary functor into a void(void*), void* pair.
- template<typename T> struct FunctorAdaptor {
- T Fn;
- static void invoke(void *Data) {
- return static_cast<FunctorAdaptor<T>*>(Data)->Fn();
- }
- typedef void Callback(void*);
- Callback *fn() { return &invoke; }
- void *arg() { return this; }
- };
-
public:
CrashRecoveryContext() : Impl(nullptr), head(nullptr) {}
~CrashRecoveryContext();
-
+
void registerCleanup(CrashRecoveryContextCleanup *cleanup);
void unregisterCleanup(CrashRecoveryContextCleanup *cleanup);
@@ -86,11 +77,9 @@
/// make as little assumptions as possible about the program state when
/// RunSafely has returned false. Clients can use getBacktrace() to retrieve
/// the backtrace of the crash on failures.
- bool RunSafely(void (*Fn)(void*), void *UserData);
- template<typename Functor>
- bool RunSafely(Functor Fn) {
- FunctorAdaptor<Functor> Adaptor = { Fn };
- return RunSafely(Adaptor.fn(), Adaptor.arg());
+ bool RunSafely(function_ref<void()> Fn);
+ bool RunSafely(void (*Fn)(void*), void *UserData) {
+ return RunSafely([&]() { Fn(UserData); });
}
/// \brief Execute the provide callback function (with the given arguments) in
@@ -98,12 +87,10 @@
/// requested stack size).
///
/// See RunSafely() and llvm_execute_on_thread().
+ bool RunSafelyOnThread(function_ref<void()>, unsigned RequestedStackSize = 0);
bool RunSafelyOnThread(void (*Fn)(void*), void *UserData,
- unsigned RequestedStackSize = 0);
- template<typename Functor>
- bool RunSafelyOnThread(Functor Fn, unsigned RequestedStackSize = 0) {
- FunctorAdaptor<Functor> Adaptor = { Fn };
- return RunSafelyOnThread(Adaptor.fn(), Adaptor.arg(), RequestedStackSize);
+ unsigned RequestedStackSize = 0) {
+ return RunSafelyOnThread([&]() { Fn(UserData); }, RequestedStackSize);
}
/// \brief Explicitly trigger a crash recovery in the current process, and
Index: lib/Support/CrashRecoveryContext.cpp
===================================================================
--- lib/Support/CrashRecoveryContext.cpp (revision 207731)
+++ lib/Support/CrashRecoveryContext.cpp (working copy)
@@ -301,7 +301,7 @@
#endif
-bool CrashRecoveryContext::RunSafely(void (*Fn)(void*), void *UserData) {
+bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
// If crash recovery is disabled, do nothing.
if (gCrashRecoveryEnabled) {
assert(!Impl && "Crash recovery context already initialized!");
@@ -313,7 +313,7 @@
}
}
- Fn(UserData);
+ Fn();
return true;
}
@@ -334,8 +334,7 @@
namespace {
struct RunSafelyOnThreadInfo {
- void (*Fn)(void*);
- void *Data;
+ function_ref<void()> Fn;
CrashRecoveryContext *CRC;
bool Result;
};
@@ -344,11 +343,11 @@
static void RunSafelyOnThread_Dispatch(void *UserData) {
RunSafelyOnThreadInfo *Info =
reinterpret_cast<RunSafelyOnThreadInfo*>(UserData);
- Info->Result = Info->CRC->RunSafely(Info->Fn, Info->Data);
+ Info->Result = Info->CRC->RunSafely(Info->Fn);
}
-bool CrashRecoveryContext::RunSafelyOnThread(void (*Fn)(void*), void *UserData,
+bool CrashRecoveryContext::RunSafelyOnThread(function_ref<void()> Fn,
unsigned RequestedStackSize) {
- RunSafelyOnThreadInfo Info = { Fn, UserData, this, false };
+ RunSafelyOnThreadInfo Info = { Fn, this, false };
llvm_execute_on_thread(RunSafelyOnThread_Dispatch, &Info, RequestedStackSize);
if (CrashRecoveryContextImpl *CRC = (CrashRecoveryContextImpl *)Impl)
CRC->setSwitchedThread();
More information about the llvm-commits
mailing list