Index: llvm/include/llvm/ADT/FunctionExtras.h =================================================================== --- llvm/include/llvm/ADT/FunctionExtras.h +++ llvm/include/llvm/ADT/FunctionExtras.h @@ -77,19 +77,28 @@ using MovePtrT = void (*)(void *LHSCallableAddr, void *RHSCallableAddr); using DestroyPtrT = void (*)(void *CallableAddr); + /// A struct to hold a single trivial callback with sufficient alignment for + /// our bitpacking. + struct alignas(8) TrivialCallback { + CallPtrT CallPtr; + }; + /// A struct we use to aggregate three callbacks when we need full set of /// operations. - struct NonTrivialCallbacks { + struct alignas(8) NonTrivialCallbacks { CallPtrT CallPtr; MovePtrT MovePtr; DestroyPtrT DestroyPtr; }; - // Now we can create a pointer union between either a direct, trivial call - // pointer and a pointer to a static struct of the call, move, and destroy - // pointers. We do this to keep the footprint in this object a single pointer - // while supporting all the necessary type-erased operation. - using CallbackPointerUnionT = PointerUnion; + // Now we can create a pointer union between either a direct trivial call + // pointer, a pointer to a static trivial call pointer, or a pointer to + // a static struct of the call, move, and destroy pointers. We do this to + // keep the footprint in this object a single pointer while supporting all + // the necessary type-erased operations with a minimum number of + // indirections. + using CallbackPointerUnionT = + PointerUnion3; // The main storage buffer. This will either have a pointer to out-of-line // storage or an inline buffer storing the callable. @@ -147,12 +156,31 @@ StorageUnion.OutOfLineStorage = {Ptr, Size, Alignment}; } + // We really want to have a well aligned function pointer if possible. That + // allows us to use a much more efficient representation when dispatching. + // + // To do this, on platforms where we expect it to be useful, we add an + // alignment attribute to try and get the function pointer to be well aligned. + // This is *not* a correctness requirement though. We will check to see if we + // ended up with adequate alignment below and use a safe approach otherwise. + // + // We currently support this on Clang and GCC-compatible compilers when + // targeting x86, PowerPC, AArch64, and ARM (but not in thumb mode as it has + // no utility then). Other architectures can be added as needed. +#if (defined(__clang__) || defined(__GNUC__)) && \ + (defined(__i386__) || defined(__x86_64__) || defined(__ppc__) || \ + defined(__aarch64__) || (defined(__arm__) && !defined(__thumb__))) +#define CALL_IMPL_ALIGNMENT __attribute__((aligned(16))) +#else +#define CALL_IMPL_ALIGNMENT +#endif template - static ReturnT CallImpl(void *CallableAddr, - AdjustedParamT... Params) { + static ReturnT CallImpl(void *CallableAddr, AdjustedParamT... Params) + CALL_IMPL_ALIGNMENT { return (*reinterpret_cast(CallableAddr))( std::forward(Params)...); } +#undef CALL_IMPL_ALIGNMENT template static void MoveImpl(void *LHSCallableAddr, void *RHSCallableAddr) noexcept { @@ -243,35 +271,42 @@ // Now move into the storage. new (CallableAddr) CallableT(std::move(Callable)); -#ifndef _MSC_VER - // See if we can create a trivial callback. - // - // This requires two things. First, we need to put a function pointer into - // a `PointerIntPair` and a `PointerUnion`. We assume that function - // pointers on almost every platform have at least 4-byte alignment which - // allows this to work, but on some platforms this appears to not hold, and - // so we need to disable this optimization on those platforms. - // - // FIXME: Currently, Windows appears to fail the asserts that check this. - // We should investigate why, and if fixed removed the #ifndef above. - // - // Second, we need the callable to be trivially moved and trivially - // destroyed so that we don't have to store type erased callbacks for those - // operations. + // See if we can create a trivial callback. We need the callable to be + // trivially moved and trivially destroyed so that we don't have to store + // type erased callbacks for those operations. // // FIXME: We should use constexpr if here and below to avoid instantiating // the non-trivial static objects when unnecessary. While the linker should // remove them, it is still wasteful. if (llvm::is_trivially_move_constructible::value && std::is_trivially_destructible::value) { - CallbackAndInlineFlag = {&CallImpl, IsInlineStorage}; + // Now see if we can embed the pointer to the trivial callback. We have to + // do this at runtime because there is no reliably way to statically + // determine (or ensure) that the function pointer we get will be + // sufficiently aligned. We take steps above to try and make this likely + // on common LLVM platforms. + CallPtrT CallImplPtr = &CallImpl; + uintptr_t CallImplAddr = reinterpret_cast(CallImplPtr); + using PtrTraits = PointerLikeTypeTraits; + uintptr_t LowBitMask = ~((uintptr_t)-1 << PtrTraits::NumLowBitsAvailable); + if ((CallImplAddr & LowBitMask) == 0) { + // The necessary bits are clear in the function pointer so we can embed + // it directly. + CallbackAndInlineFlag = {CallImplPtr, IsInlineStorage}; + return; + } + + // Otherwise, we need to create a nicely aligned object. We use a static + // variable for this because it is a trivial struct. + static TrivialCallback Callback = { CallImplPtr }; + + CallbackAndInlineFlag = {&Callback, IsInlineStorage}; return; } -#endif - // Otherwise, we need to point at an object with a vtable that contains all - // the different type erased behaviors needed. Create a static instance of - // the derived type here and then use a pointer to that. + // Otherwise, we need to point at an object that contains all the different + // type erased behaviors needed. Create a static instance of the struct type + // here and then use a pointer to that. static NonTrivialCallbacks Callbacks = { &CallImpl, &MoveImpl, &DestroyImpl}; Index: llvm/include/llvm/Support/PointerLikeTypeTraits.h =================================================================== --- llvm/include/llvm/Support/PointerLikeTypeTraits.h +++ llvm/include/llvm/Support/PointerLikeTypeTraits.h @@ -138,9 +138,12 @@ /// alignment. /// /// We assume here that functions used with this are always at least 4-byte -/// aligned. This means that, for example, thumb functions won't work or systems -/// with weird unaligned function pointers won't work. But all practical systems -/// we support satisfy this requirement. +/// aligned. This means that, for example, thumb functions won't work. There +/// are many ways that a function may end up not satisfying this alignment. +/// Users of such pointers with things like `PointerIntPair` and `PointerUnion` +/// must take care themselves that the function pointers they use are valid +/// here. This may have to rely on a runtime check in some cases due to the +/// complexity of determining this statically. template struct PointerLikeTypeTraits : FunctionPointerLikeTypeTraits<4, ReturnT (*)(ParamTs...)> {};