[PATCH] D48875: [ADT] Align unique_function::CallImpl when using clang on Windows

[Hans Wennborg via Phabricator via llvm-commits]

hans created this revision.
hans added a reviewer: chandlerc.
Herald added a subscriber: dexonsmith.

This is a follow-up to r336178.

This would be one way of working around the alignment problem.

Actually, it would be enough to just check for clang here, because even at -Os we will not emit functions with less than 4-byte alignment.

It's not very pretty though. Is this class performance sensitive enough that it's worth the complexity of packing flags into the function pointer, or could we trade size for simplicity and just store the flags in another byte?


https://reviews.llvm.org/D48875

Files:
  include/llvm/ADT/FunctionExtras.h


Index: include/llvm/ADT/FunctionExtras.h
===================================================================
--- include/llvm/ADT/FunctionExtras.h
+++ include/llvm/ADT/FunctionExtras.h
@@ -147,12 +147,19 @@
     StorageUnion.OutOfLineStorage = {Ptr, Size, Alignment};
   }
 
+#if defined(_MSC_VER) && defined(__clang__)
+  // The MS ABI does not align functions, but clang can do it.
+#define ALIGNED __attribute__((aligned(16)))
+#else
+#define ALIGNED
+#endif
   template <typename CallableT>
   static ReturnT CallImpl(void *CallableAddr,
-                          AdjustedParamT<ParamTs>... Params) {
+                          AdjustedParamT<ParamTs>... Params) ALIGNED {
     return (*reinterpret_cast<CallableT *>(CallableAddr))(
         std::forward<ParamTs>(Params)...);
   }
+#undef ALIGNED
 
   template <typename CallableT>
   static void MoveImpl(void *LHSCallableAddr, void *RHSCallableAddr) noexcept {
@@ -243,18 +250,15 @@
     // Now move into the storage.
     new (CallableAddr) CallableT(std::move(Callable));
 
-#ifndef _MSC_VER
+#if defined(_MSC_VER) && !defined(__clang__)
     // 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48875.153896.patch
Type: text/x-patch
Size: 1880 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180703/715231ba/attachment.bin>