Fix lvm::is_trivially_copyable portability issues

[serge-sans-paille]

Summary

llvm::is_trivially_copyable portability is verified at compile time using std::is_trivially_copyable as the reference implementation. Unfortunately, the latter is not available on all platforms, so introduce a proper configure check to detect if it is available on the target platform.

In a similar manner, std::is_copy_assignable is not fully supported for gcc4.9. Provide a portable (?) implementation instead.

[fedor.sergeev]

Sorry, now it fails on is_move_assignable:

] cat copyable_with_llvm.cpp
#include "llvm/Support/type_traits.h"
// code from tools/llvm-xray/xray-converter.cpp
template <typename AssociatedData> struct TrieNode {
  int FuncId;
  AssociatedData ExtraData;
};
struct StackIdData {
  // to get is_trivially_copyable instantiation from within the class
  int x [llvm::is_trivially_copyable<TrieNode<StackIdData> *>::value + 1];
};
] g++ -std=c++11 -I ../../llvm/include -I include copyable_with_llvm.cpp 
copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
/gcc/include/c++/4.9.2/type_traits:1183:45:   required by substitution of ‘template<class _Tp1, class _Up1, class> static std::true_type std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&; _Up1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&; <template-parameter-1-3> = <missing>]’
/gcc/include/c++/4.9.2/type_traits:1192:30:   required from ‘class std::__is_assignable_helper<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1197:12:   required from ‘struct std::is_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1227:12:   required from ‘struct std::__is_move_assignable_impl<llvm::detail::trivial_helper<TrieNode<StackIdData>*>, true>’
/gcc/include/c++/4.9.2/type_traits:1233:12:   required from ‘struct std::is_move_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:157:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_move_assign’
../../llvm/include/llvm/Support/type_traits.h:170:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
copyable_with_llvm.cpp:9:64:   required from here
copyable_with_llvm.cpp:5:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
copyable_with_llvm.cpp:7:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

And yes, the same failure on the actual llvm build on xray-converter.cpp.

[fedor.sergeev]

include/llvm/Support/type_traits.h

150

Somehow this declval<F> thing does not look right. Doesnt it have F&& type?
Besides, the result of assignment might as well be a reference, which will reject forming a pointer on its decltype.

[serge-sans-paille]

A few comments missing but I'd like to know if this now works correctly on gcc 4.9, @fedor.sergeev , @Anastasia and @sylvestre.ledru . I successfully passed test on a dedicated setup so I guess it's ok...

[fedor.sergeev]

it does *not* work on my shorter reproducer:

] g++ -std=c++11 -I ../../llvm/include -I include ../../llvm/copyable_with_llvm.cpp 
../../llvm/copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
../../llvm/include/llvm/Support/type_traits.h:118:34:   required by substitution of ‘template<class T> using IsCopyAssignableImpl = decltype ((declval<T&>)()=(declval<const T&>)()) [with T = llvm::detail::trivial_helper<TrieNode<StackIdData>*>]’
../../llvm/include/llvm/Support/type_traits.h:114:8:   required from ‘struct llvm::detail::IsDetected<llvm::detail::IsCopyAssignableImpl, llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:149:8:   required from ‘struct llvm::is_copy_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:174:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_copy_assign’
../../llvm/include/llvm/Support/type_traits.h:195:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
../../llvm/copyable_with_llvm.cpp:12:64:   required from here
../../llvm/copyable_with_llvm.cpp:7:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
../../llvm/copyable_with_llvm.cpp:10:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

Started full build, will report as soon as it is ready.

[fedor.sergeev]

In D57018#1365764, @fedor.sergeev wrote:

Started full build, will report as soon as it is ready.

On xray-converter.cpp fails the same way as on my shorter reproducer.

[fedor.sergeev]

Both the small reproducer and full LLVM-only builds/tests passed with my gcc 4.9.
LGTM

[xbolva00]

cmake/config-ix.cmake

328

Typo

[serge-sans-paille]

Slightly cleaner implementation, added comments and tested with polly.

[TWeaver]

Hi there,

we're seeing build failures on our internal CI as a result of the issue this patch aims to fix.

hope I don't seem too pushy but, are there any plans to land this in the next few hours?

we'd like to see our CI back up and running as soon as possible to avoid any further headaches.

Thanks in advance
Tom W

[serge-sans-paille]

Yeah, I do a final test round and I'll push that one. It's a very touchy piece of code, so I don't want to make all builders fail again...

[uabelho]

We've been having issues with std::is_trivially_copyable too, and it seems not even this patch solves the problems for us.

I'm running an Ubuntu 14.04 machine with clang 3.6.0 and libstdc++-4.8-dev and now I get the following when I compile on trunk:

In file included from ../utils/TableGen/CodeGenSchedule.cpp:14:
In file included from ../utils/TableGen/CodeGenSchedule.h:18:
In file included from ../include/llvm/ADT/DenseMap.h:16:
In file included from ../include/llvm/ADT/DenseMapInfo.h:16:
In file included from ../include/llvm/ADT/ArrayRef.h:12:
In file included from ../include/llvm/ADT/Hashing.h:48:
In file included from ../include/llvm/Support/Host.h:16:
In file included from ../include/llvm/ADT/StringMap.h:16:
In file included from ../include/llvm/ADT/StringRef.h:12:
In file included from ../include/llvm/ADT/STLExtras.h:19:
In file included from ../include/llvm/ADT/Optional.h:21:
../include/llvm/Support/type_traits.h:127:57: error: object of type 'llvm::detail::trivial_helper<std::pair<void *, unsigned long> >' cannot

  be assigned because its copy assignment operator is implicitly deleted
static auto get(F*) -> decltype(std::declval<T &>() = std::declval<const T &>(), std::true_type{});
                                                    ^

../include/llvm/Support/type_traits.h:162:7: note: in instantiation of template class

'llvm::is_copy_assignable<llvm::detail::trivial_helper<std::pair<void *, unsigned long> > >' requested here
is_copy_assignable<detail::trivial_helper<T>>::value;
^

../include/llvm/ADT/SmallVector.h:184:30: note: in instantiation of template class 'llvm::is_trivially_copyable<std::pair<void *, unsigned

long> >' requested here

template <typename T, bool = is_trivially_copyable<T>::value>

^

../include/llvm/ADT/SmallVector.h:321:32: note: in instantiation of default argument for 'SmallVectorTemplateBase<std::pair<void *, unsigned

long> >' required here

class SmallVectorImpl : public SmallVectorTemplateBase<T> {

^~~~~~~~~~~~~~~~~~~~~~~~~~

../include/llvm/ADT/SmallVector.h:845:28: note: in instantiation of template class 'llvm::SmallVectorImpl<std::pair<void *, unsigned long> >'

requested here

class SmallVector : public SmallVectorImpl<T>, SmallVectorStorage<T, N> {

^

../include/llvm/Support/Allocator.h:373:45: note: in instantiation of template class 'llvm::SmallVector<std::pair<void *, unsigned long>, 0>'

    requested here
SmallVector<std::pair<void *, size_t>, 0> CustomSizedSlabs;
                                          ^

../include/llvm/Support/type_traits.h:96:7: note: copy assignment operator of 'trivial_helper<std::pair<void *, unsigned long> >' is

  implicitly deleted because variant field 't' has a non-trivial copy assignment operator
T t;
  ^

[uabelho]

Just saw build-bots failed the same way

[TWeaver]

seems like the best thing to do here is revert the original commit that broke the bots in the first place?

[TWeaver]

and thanks for the patch either way, nice to know we're not in this alone.

[TWeaver]

Slight fix for r351820 seems to have done the trick, many thanks!

[serge-sans-paille]

@uabelho top-of-tree should be fine now.

[uabelho]

In D57018#1366287, @serge-sans-paille wrote:

@uabelho top-of-tree should be fine now.

Yep, thanks!

[v.g.vassilev]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

Hi @serge-sans-paille, we hit this assert when compiling llvm9 on windows (latest version (v16.8.2) of VS 2019):

llvm/Support/type_traits.h(186,23): error C2338: inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable (compiling source file RTDyldMemoryManager.cpp)
llvm/ADT/SmallVector.h(315,32): error C2976: 'llvm::SmallVectorTemplateBase': too few template arguments (compiling source file RTDyldMemoryManager.cpp)

Do you know if this is a bug in msvc or we need to improve something in that area?

[rogerjarrett]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

I am experiencing the same error compiling LLVM 12.0.0 with VC19 version 14.27.29110 any updates ?

[rogerjarrett]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

Mistake on my part:I am experiencing the same error compiling LLVM 10.0.1 with VC19 version 14.27.29110 any updates ? Appears to be related to MSVC compiler version

cmake/config-ix.cmake

if( LLVM_ENABLE_FFI )
   list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES ${FFI_INCLUDE_PATH})
   list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${FFI_LIBRARY_PATH})
 else()
   unset(HAVE_FFI_FFI_H CACHE)
   unset(HAVE_FFI_H CACHE)
   unset(HAVE_FFI_CALL CACHE)
 endif( LLVM_ENABLE_FFI )
 
+# Wether we can use std::is_triviallyc_opyable to verify llvm::is_trivially_copyable
+CHECK_CXX_SOURCE_COMPILES("
+#include <type_traits>
+struct T { int val; };
+static_assert(std::is_trivially_copyable<T>::value, \"ok\");
+int main() { return 0;}
+" HAVE_STD_IS_TRIVIALLY_COPYABLE)
+
+
 # Define LLVM_HAS_ATOMICS if gcc or MSVC atomic builtins are supported.
 include(CheckAtomic)
 
 if( LLVM_ENABLE_PIC )
   set(ENABLE_PIC 1)
 else()
   set(ENABLE_PIC 0)
   check_cxx_compiler_flag("-fno-pie" SUPPORTS_NO_PIE_FLAG)

include/llvm/ADT/PointerIntPair.h

 //===- llvm/ADT/PointerIntPair.h - Pair for pointer and int -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the PointerIntPair class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_POINTERINTPAIR_H
 #define LLVM_ADT_POINTERINTPAIR_H
 
 #include "llvm/Support/PointerLikeTypeTraits.h"
+#include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <cstdint>
 #include <limits>
 
 namespace llvm {
 
 template <typename T> struct DenseMapInfo;
 template <typename PointerT, unsigned IntBits, typename PtrTraits>
bool operator<=(const PointerIntPair &RHS) const {
     return Value <= RHS.Value;
   }
 
   bool operator>=(const PointerIntPair &RHS) const {
     return Value >= RHS.Value;
   }
 };
 
+template <typename PointerTy, unsigned IntBits, typename IntType,
+          typename PtrTraits,
+          typename Info>
+struct is_trivially_copyable<PointerIntPair<PointerTy, IntBits, IntType, PtrTraits, Info>> : std::true_type {
+#ifdef HAVE_STD_IS_TRIVIALLY_COPYABLE
+  static_assert(std::is_trivially_copyable<PointerIntPair<PointerTy, IntBits, IntType, PtrTraits, Info>>::value,
+                "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
+#endif
+};
+
+
 template <typename PointerT, unsigned IntBits, typename PtrTraits>
 struct PointerIntPairInfo {
   static_assert(PtrTraits::NumLowBitsAvailable <
                     std::numeric_limits<uintptr_t>::digits,
                 "cannot use a pointer type that has all bits free");
   static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
                 "PointerIntPair with integer size too large for pointer");
   enum : uintptr_t {

include/llvm/Config/config.h.cmake

 #cmakedefine PACKAGE_VERSION "${PACKAGE_VERSION}"
 
 /* Define to the vendor of this package. */
 #cmakedefine PACKAGE_VENDOR "${PACKAGE_VENDOR}"
 
 /* Define as the return type of signal handlers (`int' or `void'). */
 #cmakedefine RETSIGTYPE ${RETSIGTYPE}
 
+/* Define if std::is_trivially_copyable is supported */
+#cmakedefine HAVE_STD_IS_TRIVIALLY_COPYABLE ${HAVE_STD_IS_TRIVIALLY_COPYABLE}
+
 /* Define to a function implementing stricmp */
 #cmakedefine stricmp ${stricmp}
 
 /* Define to a function implementing strdup */
 #cmakedefine strdup ${strdup}
 
 /* Whether GlobalISel rule coverage is being collected */
 #cmakedefine01 LLVM_GISEL_COV_ENABLED
 
 /* Define to the default GlobalISel coverage file prefix */
 #cmakedefine LLVM_GISEL_COV_PREFIX "${LLVM_GISEL_COV_PREFIX}"
 
 #endif

include/llvm/Support/type_traits.h

template<typename T> union move_construction_triviality_helper {
     move_construction_triviality_helper(move_construction_triviality_helper&&) = default;
     ~move_construction_triviality_helper() = default;
 };
 
 template<class T>
 union trivial_helper {
     T t;
 };
+
+
+template <typename... Ts> struct VoidTImpl { using type = void; };
+
+template <class Enabler, template <class...> class Op, class... Args>
+struct IsDetectedImpl {
+  using type = std::false_type;
+};
+
+template <template <class...> class Op, class... Args>
+struct IsDetectedImpl<typename VoidTImpl<Op<Args...>>::type, Op, Args...> {
+  using type = std::true_type;
+};
+
+
+template <template <class...> class Op, class... Args>
+struct IsDetected : IsDetectedImpl<void, Op, Args...>::type {};
+
+template <typename T>
+using IsCopyAssignableImpl =
+    decltype(std::declval<T &>() = std::declval<const T &>());
+
+template <typename T>
+using IsMoveAssignableImpl =
+    decltype(std::declval<T &>() = std::declval<T &&>());
 } // end namespace detail
 
 /// An implementation of `std::is_trivially_copy_constructible` since we have
 /// users with STLs that don't yet include it.
 template <typename T>
 struct is_trivially_copy_constructible
     : std::is_copy_constructible<
           ::llvm::detail::copy_construction_triviality_helper<T>> {};
 template <typename T>
 struct is_trivially_copy_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_copy_constructible<T &&> : std::false_type {};
 
 /// An implementation of `std::is_trivially_move_constructible` since we have
 /// users with STLs that don't yet include it.
 template <typename T>
 struct is_trivially_move_constructible
     : std::is_move_constructible<
           ::llvm::detail::move_construction_triviality_helper<T>> {};
 template <typename T>
 struct is_trivially_move_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_move_constructible<T &&> : std::true_type {};
 
+
+template <typename T>
+struct is_copy_assignable : detail::IsDetected<detail::IsCopyAssignableImpl, T> {};
+
+template <typename T>
+struct is_move_assignable : detail::IsDetected<detail::IsMoveAssignableImpl, T> {};
+
 // An implementation of `std::is_trivially_copyable` since STL version
 // is not equally supported by all compilers, especially GCC 4.9.
 // Uniform implementation of this trait is important for ABI compatibility
 // as it has an impact on SmallVector's ABI (among others).
 template <typename T>
 class is_trivially_copyable {
 
   // copy constructors
   static constexpr bool has_trivial_copy_constructor =
       std::is_copy_constructible<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_copy_constructor =
       !std::is_copy_constructible<T>::value;
 
   // move constructors
   static constexpr bool has_trivial_move_constructor =
       std::is_move_constructible<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_move_constructor =
       !std::is_move_constructible<T>::value;
 
   // copy assign
   static constexpr bool has_trivial_copy_assign =
-      std::is_copy_assignable<detail::trivial_helper<T>>::value;
+      is_copy_assignable<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_copy_assign =
-      !std::is_copy_assignable<T>::value;
+      !is_copy_assignable<T>::value;
 
   // move assign
   static constexpr bool has_trivial_move_assign =
-      std::is_move_assignable<detail::trivial_helper<T>>::value;
+      is_move_assignable<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_move_assign =
-      !std::is_move_assignable<T>::value;
+      !is_move_assignable<T>::value;
 
   // destructor
   static constexpr bool has_trivial_destructor =
       std::is_destructible<detail::trivial_helper<T>>::value;
 
   public:
 
   static constexpr bool value =
       has_trivial_destructor &&
       (has_deleted_move_assign || has_trivial_move_assign) &&
       (has_deleted_move_constructor || has_trivial_move_constructor) &&
       (has_deleted_copy_assign || has_trivial_copy_assign) &&
       (has_deleted_copy_constructor || has_trivial_copy_constructor);
 
-#if (__has_feature(is_trivially_copyable) || (defined(__GNUC__) && __GNUC__ >= 5))
-  static_assert(value == std::is_trivially_copyable<T>::value, "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
+#ifdef HAVE_STD_IS_TRIVIALLY_COPYABLE
+  static_assert(value == std::is_trivially_copyable<T>::value,
+                "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
 #endif
 };
+template <typename T>
+class is_trivially_copyable<T*> : public std::true_type {
+};
 
 
 } // end namespace llvm
 
 // If the compiler supports detecting whether a class is final, define
 // an LLVM_IS_FINAL macro. If it cannot be defined properly, this
 // macro will be left undefined.
 #if __cplusplus >= 201402L || defined(_MSC_VER)

tools/llvm-xray/trie-node.h

 #include "llvm/ADT/SmallVector.h"
 
 /// A type to represent a trie of invocations. It is useful to construct a
 /// graph of these nodes from reading an XRay trace, such that each function
 /// call can be placed in a larger context.
 ///
 /// The template parameter allows users of the template to attach their own
 /// data elements to each node in the invocation graph.
+template <typename AssociatedData> struct TrieNode;
+namespace llvm {
+template <typename AssociatedData>
+struct is_triviallly_copyable {
+  static constexpr bool value = is_trivially_copyable<AssociatedData>::value;
+#ifdef HAVE_STD_IS_TRIVIALLY_COPYABLE
+  static_assert(value == std::is_trivially_copyable<T>::value,
+                "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
+#endif
+};
+}
+
 template <typename AssociatedData> struct TrieNode {
   /// The function ID.
   int32_t FuncId;
 
   /// The caller of this function.
   TrieNode<AssociatedData> *Parent;
 
   /// The callees from this function.