Replace llvm::isPodLike<...> by llvm::is_trivially_copyable<...>

As noted in https://bugs.llvm.org/show_bug.cgi?id=36651, the specialization for
isPodLike<std::pair<...>> did not match the expectation of
std::is_trivially_copyable which makes the memcpy optimization invalid.

This patch renames the llvm::isPodLike trait into llvm::is_trivially_copyable.
Unfortunately std::is_trivially_copyable is not portable across compiler / STL
versions. So a portable version is provided too.

Note that the following specialization were invalid:

    std::pair<T0, T1>
    llvm::Optional<T>

Tests have been added to assert that former specialization are respected by the
standard usage of llvm::is_trivially_copyable, and that when a decent version
of std::is_trivially_copyable is available, llvm::is_trivially_copyable is
compared to std::is_trivially_copyable.

As of this patch, llvm::Optional is no longer considered trivially copyable,
even if T is. This is to be fixed in a later patch, as it has impact on a
long-running bug (see r347004)

Note that GCC warns about this UB, but this got silented by https://reviews.llvm.org/D50296.

Differential Revision: https://reviews.llvm.org/D54472



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351701 91177308-0d34-0410-b5e6-96231b3b80d8

docs/ProgrammersManual.rst

@@ -1456,7 +1456,7 @@ SmallVector has grown a few other minor advantages over std::vector, causing
 #. std::vector is exception-safe, and some implementations have pessimizations
    that copy elements when SmallVector would move them.
 
-#. SmallVector understands ``isPodLike<Type>`` and uses realloc aggressively.
+#. SmallVector understands ``llvm::is_trivially_copyable<Type>`` and uses realloc aggressively.
 
 #. Many LLVM APIs take a SmallVectorImpl as an out parameter (see the note
    below).

include/llvm/ADT/ArrayRef.h

@@ -525,12 +525,6 @@ namespace llvm {
 
   /// @}
 
-  // ArrayRefs can be treated like a POD type.
-  template <typename T> struct isPodLike;
-  template <typename T> struct isPodLike<ArrayRef<T>> {
-    static const bool value = true;
-  };
-
   template <typename T> hash_code hash_value(ArrayRef<T> S) {
     return hash_combine_range(S.begin(), S.end());
   }

include/llvm/ADT/DenseMap.h

@@ -145,7 +145,8 @@ public:
     }
 
     const KeyT EmptyKey = getEmptyKey(), TombstoneKey = getTombstoneKey();
-    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value) {
+    if (is_trivially_copyable<KeyT>::value &&
+        is_trivially_copyable<ValueT>::value) {
       // Use a simpler loop when these are trivial types.
       for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)
         P->getFirst() = EmptyKey;
@@ -421,7 +422,8 @@ protected:
     setNumEntries(other.getNumEntries());
     setNumTombstones(other.getNumTombstones());
 
-    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value)
+    if (is_trivially_copyable<KeyT>::value &&
+        is_trivially_copyable<ValueT>::value)
       memcpy(reinterpret_cast<void *>(getBuckets()), other.getBuckets(),
              getNumBuckets() * sizeof(BucketT));
     else

include/llvm/ADT/ImmutableList.h

@@ -241,10 +241,6 @@ template<typename T> struct DenseMapInfo<ImmutableList<T>> {
   }
 };
 
-template <typename T> struct isPodLike;
-template <typename T>
-struct isPodLike<ImmutableList<T>> { static const bool value = true; };
-
 } // end namespace llvm
 
 #endif // LLVM_ADT_IMMUTABLELIST_H

include/llvm/ADT/Optional.h

@@ -30,7 +30,7 @@ class raw_ostream;
 
 namespace optional_detail {
 /// Storage for any type.
-template <typename T, bool = isPodLike<T>::value> struct OptionalStorage {
+template <typename T, bool = is_trivially_copyable<T>::value> struct OptionalStorage {
   AlignedCharArrayUnion<T> storage;
   bool hasVal = false;
 
@@ -185,11 +185,6 @@ public:
 #endif
 };
 
-template <typename T> struct isPodLike<Optional<T>> {
-  // An Optional<T> is pod-like if T is.
-  static const bool value = isPodLike<T>::value;
-};
-
 template <typename T, typename U>
 bool operator==(const Optional<T> &X, const Optional<U> &Y) {
   if (X && Y)

include/llvm/ADT/PointerIntPair.h

@@ -175,12 +175,6 @@ struct PointerIntPairInfo {
   }
 };
 
-template <typename T> struct isPodLike;
-template <typename PointerTy, unsigned IntBits, typename IntType>
-struct isPodLike<PointerIntPair<PointerTy, IntBits, IntType>> {
-  static const bool value = true;
-};
-
 // Provide specialization of DenseMapInfo for PointerIntPair.
 template <typename PointerTy, unsigned IntBits, typename IntType>
 struct DenseMapInfo<PointerIntPair<PointerTy, IntBits, IntType>> {

include/llvm/ADT/SmallVector.h

@@ -179,9 +179,9 @@ public:
   }
 };
 
-/// SmallVectorTemplateBase<isPodLike = false> - This is where we put method
+/// SmallVectorTemplateBase<TriviallyCopyable = false> - This is where we put method
 /// implementations that are designed to work with non-POD-like T's.
-template <typename T, bool = isPodLike<T>::value>
+template <typename T, bool = is_trivially_copyable<T>::value>
 class SmallVectorTemplateBase : public SmallVectorTemplateCommon<T> {
 protected:
   SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon<T>(Size) {}
@@ -235,8 +235,8 @@ public:
 };
 
 // Define this out-of-line to dissuade the C++ compiler from inlining it.
-template <typename T, bool isPodLike>
-void SmallVectorTemplateBase<T, isPodLike>::grow(size_t MinSize) {
+template <typename T, bool TriviallyCopyable>
+void SmallVectorTemplateBase<T, TriviallyCopyable>::grow(size_t MinSize) {
   if (MinSize > UINT32_MAX)
     report_bad_alloc_error("SmallVector capacity overflow during allocation");
 
@@ -259,9 +259,8 @@ void SmallVectorTemplateBase<T, isPodLike>::grow(size_t MinSize) {
   this->Capacity = NewCapacity;
 }
 
-
-/// SmallVectorTemplateBase<isPodLike = true> - This is where we put method
-/// implementations that are designed to work with POD-like T's.
+/// SmallVectorTemplateBase<TriviallyCopyable = true> - This is where we put
+/// method implementations that are designed to work with POD-like T's.
 template <typename T>
 class SmallVectorTemplateBase<T, true> : public SmallVectorTemplateCommon<T> {
 protected:
@@ -330,7 +329,7 @@ public:
 protected:
   // Default ctor - Initialize to empty.
   explicit SmallVectorImpl(unsigned N)
-      : SmallVectorTemplateBase<T, isPodLike<T>::value>(N) {}
+      : SmallVectorTemplateBase<T>(N) {}
 
 public:
   SmallVectorImpl(const SmallVectorImpl &) = delete;

include/llvm/ADT/StringRef.h

@@ -927,10 +927,6 @@ namespace llvm {
   LLVM_NODISCARD
   hash_code hash_value(StringRef S);
 
-  // StringRefs can be treated like a POD type.
-  template <typename T> struct isPodLike;
-  template <> struct isPodLike<StringRef> { static const bool value = true; };
-
 } // end namespace llvm
 
 #endif // LLVM_ADT_STRINGREF_H

include/llvm/ADT/bit.h

@@ -40,11 +40,11 @@ template <typename To, typename From
           , typename = typename std::enable_if<__is_trivially_copyable(To)>::type
           , typename = typename std::enable_if<__is_trivially_copyable(From)>::type
 #else
-  // This case is GCC 4.x. clang with libc++ or libstdc++ never get here. Unlike
-  // llvm/Support/type_traits.h's isPodLike we don't want to provide a
-  // good-enough answer here: developers in that configuration will hit
-  // compilation failures on the bots instead of locally. That's acceptable
-  // because it's very few developers, and only until we move past C++11.
+// This case is GCC 4.x. clang with libc++ or libstdc++ never get here. Unlike
+// llvm/Support/type_traits.h's is_trivially_copyable we don't want to
+// provide a good-enough answer here: developers in that configuration will hit
+// compilation failures on the bots instead of locally. That's acceptable
+// because it's very few developers, and only until we move past C++11.
 #endif
 >
 inline To bit_cast(const From &from) noexcept {

include/llvm/Analysis/BlockFrequencyInfoImpl.h

@@ -159,10 +159,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, BlockMass X) {
 
 } // end namespace bfi_detail
 
-template <> struct isPodLike<bfi_detail::BlockMass> {
-  static const bool value = true;
-};
-
 /// Base class for BlockFrequencyInfoImpl
 ///
 /// BlockFrequencyInfoImplBase has supporting data structures and some
@@ -186,9 +182,9 @@ public:
   struct BlockNode {
     using IndexType = uint32_t;
 
-    IndexType Index = std::numeric_limits<uint32_t>::max();
+    IndexType Index;
 
-    BlockNode() = default;
+    BlockNode() : Index(std::numeric_limits<uint32_t>::max()) {}
     BlockNode(IndexType Index) : Index(Index) {}
 
     bool operator==(const BlockNode &X) const { return Index == X.Index; }

include/llvm/Bitcode/BitCodes.h

@@ -159,8 +159,6 @@ public:
 
 };
 
-template <> struct isPodLike<BitCodeAbbrevOp> { static const bool value=true; };
-
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
 /// abbreviation allows a complex record that has redundancy to be stored in a
 /// specialized format instead of the fully-general, fully-vbr, format.

include/llvm/CodeGen/DIE.h

@@ -799,7 +799,7 @@ class DIEUnit {
   const uint16_t Version; /// The Dwarf version number for this unit.
   const uint8_t AddrSize; /// The size in bytes of an address for this unit.
 protected:
-  ~DIEUnit() = default;
+  virtual ~DIEUnit() = default;
 
 public:
   DIEUnit(uint16_t Version, uint8_t AddrSize, dwarf::Tag UnitTag);

include/llvm/CodeGen/RegisterPressure.h

@@ -131,10 +131,6 @@ public:
   }
 };
 
-template <> struct isPodLike<PressureChange> {
-   static const bool value = true;
-};
-
 /// List of PressureChanges in order of increasing, unique PSetID.
 ///
 /// Use a small fixed number, because we can fit more PressureChanges in an

include/llvm/CodeGen/ScheduleDAG.h

@@ -238,9 +238,6 @@ class TargetRegisterInfo;
     void dump(const TargetRegisterInfo *TRI = nullptr) const;
   };
 
-  template <>
-  struct isPodLike<SDep> { static const bool value = true; };
-
   /// Scheduling unit. This is a node in the scheduling DAG.
   class SUnit {
   private:

include/llvm/CodeGen/SelectionDAGNodes.h

@@ -231,7 +231,6 @@ template<> struct DenseMapInfo<SDValue> {
     return LHS == RHS;
   }
 };
-template <> struct isPodLike<SDValue> { static const bool value = true; };
 
 /// Allow casting operators to work directly on
 /// SDValues as if they were SDNode*'s.

include/llvm/CodeGen/SlotIndexes.h

@@ -301,8 +301,6 @@ class raw_ostream;
     }
   };
 
-  template <> struct isPodLike<SlotIndex> { static const bool value = true; };
-
   inline raw_ostream& operator<<(raw_ostream &os, SlotIndex li) {
     li.print(os);
     return os;

include/llvm/CodeGen/TargetPassConfig.h

@@ -74,9 +74,6 @@ public:
   }
 };
 
-template <> struct isPodLike<IdentifyingPassPtr> {
-  static const bool value = true;
-};
 
 /// Target-Independent Code Generator Pass Configuration Options.
 ///

include/llvm/IR/CFG.h

@@ -237,10 +237,6 @@ public:
   }
 };
 
-template <typename T, typename U> struct isPodLike<SuccIterator<T, U>> {
-  static const bool value = isPodLike<T>::value;
-};
-
 using succ_iterator = SuccIterator<Instruction, BasicBlock>;
 using succ_const_iterator = SuccIterator<const Instruction, const BasicBlock>;
 using succ_range = iterator_range<succ_iterator>;

include/llvm/IR/ValueHandle.h

@@ -308,15 +308,6 @@ struct DenseMapInfo<AssertingVH<T>> {
   }
 };
 
-template <typename T>
-struct isPodLike<AssertingVH<T>> {
-#ifdef NDEBUG
-  static const bool value = true;
-#else
-  static const bool value = false;
-#endif
-};
-
 /// Value handle that tracks a Value across RAUW.
 ///
 /// TrackingVH is designed for situations where a client needs to hold a handle
@@ -548,14 +539,6 @@ template <typename T> struct DenseMapInfo<PoisoningVH<T>> {
   }
 };
 
-template <typename T> struct isPodLike<PoisoningVH<T>> {
-#ifdef NDEBUG
-  static const bool value = true;
-#else
-  static const bool value = false;
-#endif
-};
-
 } // end namespace llvm
 
 #endif // LLVM_IR_VALUEHANDLE_H

include/llvm/MC/MCInst.h

@@ -153,8 +153,6 @@ public:
   bool evaluateAsConstantImm(int64_t &Imm) const;
 };
 
-template <> struct isPodLike<MCOperand> { static const bool value = true; };
-
 /// Instances of this class represent a single low-level machine
 /// instruction.
 class MCInst {

include/llvm/Support/ScaledNumber.h

@@ -886,10 +886,6 @@ template <class DigitsT> void ScaledNumber<DigitsT>::shiftRight(int32_t Shift) {
   Digits >>= Shift;
 }
 
-template <typename T> struct isPodLike;
-template <typename T> struct isPodLike<ScaledNumber<T>> {
-  static const bool value = true;
-};
 
 } // end namespace llvm
 

include/llvm/Support/type_traits.h

@@ -24,35 +24,6 @@
 
 namespace llvm {
 
-/// isPodLike - This is a type trait that is used to determine whether a given
-/// type can be copied around with memcpy instead of running ctors etc.
-template <typename T>
-struct isPodLike {
-  // std::is_trivially_copyable is available in libc++ with clang, libstdc++
-  // that comes with GCC 5.  MSVC 2015 and newer also have
-  // std::is_trivially_copyable.
-#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) ||      \
-    (defined(__GNUC__) && __GNUC__ >= 5) || defined(_MSC_VER)
-  // If the compiler supports the is_trivially_copyable trait use it, as it
-  // matches the definition of isPodLike closely.
-  static const bool value = std::is_trivially_copyable<T>::value;
-#elif __has_feature(is_trivially_copyable)
-  // Use the internal name if the compiler supports is_trivially_copyable but we
-  // don't know if the standard library does. This is the case for clang in
-  // conjunction with libstdc++ from GCC 4.x.
-  static const bool value = __is_trivially_copyable(T);
-#else
-  // If we don't know anything else, we can (at least) assume that all non-class
-  // types are PODs.
-  static const bool value = !std::is_class<T>::value;
-#endif
-};
-
-// std::pair's are pod-like if their elements are.
-template<typename T, typename U>
-struct isPodLike<std::pair<T, U>> {
-  static const bool value = isPodLike<T>::value && isPodLike<U>::value;
-};
 
 /// Metafunction that determines whether the given type is either an
 /// integral type or an enumeration type, including enum classes.
@@ -119,6 +90,11 @@ 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;
+};
 } // end namespace detail
 
 /// An implementation of `std::is_trivially_copy_constructible` since we have
@@ -143,6 +119,56 @@ struct is_trivially_move_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_move_constructible<T &&> : std::true_type {};
 
+// 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;
+  static constexpr bool has_deleted_copy_assign =
+      !std::is_copy_assignable<T>::value;
+
+  // move assign
+  static constexpr bool has_trivial_move_assign =
+      std::is_move_assignable<detail::trivial_helper<T>>::value;
+  static constexpr bool has_deleted_move_assign =
+      !std::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");
+#endif
+};
+
+
 } // end namespace llvm
 
 // If the compiler supports detecting whether a class is final, define

lib/Transforms/Scalar/SROA.cpp

@@ -221,13 +221,6 @@ public:
 
 } // end anonymous namespace
 
-namespace llvm {
-
-template <typename T> struct isPodLike;
-template <> struct isPodLike<Slice> { static const bool value = true; };
-
-} // end namespace llvm
-
 /// Representation of the alloca slices.
 ///
 /// This class represents the slices of an alloca which are formed by its

tools/llvm-diff/DifferenceEngine.cpp

@@ -67,7 +67,7 @@ public:
     unsigned NewSize = Storage.size() - 1;
     if (NewSize) {
       // Move the slot at the end to the beginning.
-      if (isPodLike<T>::value)
+      if (is_trivially_copyable<T>::value)
         Storage[0] = Storage[NewSize];
       else
         std::swap(Storage[0], Storage[NewSize]);

unittests/ADT/ArrayRefTest.cpp

@@ -248,4 +248,7 @@ TEST(ArrayRefTest, makeArrayRef) {
   EXPECT_TRUE(AR2.equals(AR2Ref));
 }
 
+static_assert(is_trivially_copyable<ArrayRef<int>>::value,
+              "trivially copyable");
+
 } // end anonymous namespace

unittests/ADT/ImmutableListTest.cpp

@@ -267,4 +267,7 @@ TEST_F(ImmutableListTest, LongListOrderingTest) {
   ASSERT_EQ(6, i);
 }
 
+static_assert(is_trivially_copyable<ImmutableList<Wrapper<long>>>::value,
+              "trivially copyable");
+
 } // namespace

unittests/ADT/PointerIntPairTest.cpp

@@ -61,6 +61,9 @@ TEST(PointerIntPairTest, GetSet) {
   Pair2.setPointerAndInt(&s, E::Case3);
   EXPECT_EQ(&s, Pair2.getPointer());
   EXPECT_EQ(E::Case3, Pair2.getInt());
+
+  static_assert(is_trivially_copyable<PointerIntPair<S *, 2, E>>::value,
+                "trivially copyable");
 }
 
 TEST(PointerIntPairTest, DefaultInitialize) {
@@ -96,6 +99,11 @@ TEST(PointerIntPairTest, ManyUnusedBits) {
 
   EXPECT_EQ(FixnumPointerTraits::NumLowBitsAvailable - 1,
             PointerLikeTypeTraits<decltype(pair)>::NumLowBitsAvailable);
+
+  static_assert(
+      is_trivially_copyable<
+          PointerIntPair<Fixnum31, 1, bool, FixnumPointerTraits>>::value,
+      "trivially copyable");
 }
 
 } // end anonymous namespace

unittests/ADT/StringRefTest.cpp

@@ -1061,4 +1061,6 @@ TEST(StringRefTest, StringLiteral) {
   EXPECT_EQ(StringRef("Bar"), Strings[1]);
 }
 
+static_assert(is_trivially_copyable<StringRef>::value, "trivially copyable");
+
 } // end anonymous namespace

unittests/Analysis/BlockFrequencyInfoTest.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/BlockFrequencyInfoImpl.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/AsmParser/Parser.h"
@@ -90,5 +91,8 @@ TEST_F(BlockFrequencyInfoTest, Basic) {
   EXPECT_EQ(BFI.getBlockFreq(BB3).getFrequency(), BB3Freq);
 }
 
+static_assert(is_trivially_copyable<bfi_detail::BlockMass>::value,
+              "trivially copyable");
+
 } // end anonymous namespace
 } // end namespace llvm

unittests/Bitcode/BitstreamReaderTest.cpp

@@ -147,4 +147,7 @@ TEST(BitstreamReaderTest, shortRead) {
   }
 }
 
+static_assert(is_trivially_copyable<BitCodeAbbrevOp>::value,
+              "trivially copyable");
+
 } // end anonymous namespace

unittests/CodeGen/CMakeLists.txt

@@ -19,6 +19,7 @@ add_llvm_unittest(CodeGenTests
   MachineInstrTest.cpp
   MachineOperandTest.cpp
   ScalableVectorMVTsTest.cpp
+  TypeTraitsTest.cpp
   )
 
 add_subdirectory(GlobalISel)

unittests/CodeGen/MachineInstrTest.cpp

@@ -272,4 +272,6 @@ TEST(MachineInstrPrintingTest, DebugLocPrinting) {
       StringRef(OS.str()).endswith("filename:1:5"));
 }
 
+static_assert(is_trivially_copyable<MCOperand>::value, "trivially copyable");
+
 } // end namespace

unittests/IR/CFGBuilder.cpp

@@ -8,6 +8,7 @@
 
 #include "CFGBuilder.h"
 
+#include "llvm/IR/CFG.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Support/Debug.h"
@@ -265,3 +266,11 @@ TEST(CFGBuilder, Rebuild) {
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("c")->getTerminator()));
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("d")->getTerminator()));
 }
+
+static_assert(is_trivially_copyable<succ_iterator>::value,
+              "trivially copyable");
+static_assert(is_trivially_copyable<succ_const_iterator>::value,
+              "trivially copyable");
+static_assert(is_trivially_copyable<succ_range>::value, "trivially copyable");
+static_assert(is_trivially_copyable<succ_const_range>::value,
+              "trivially copyable");

unittests/Support/ScaledNumberTest.cpp

@@ -562,4 +562,7 @@ TEST(ScaledNumberHelpersTest, toIntBug) {
   EXPECT_EQ(1u, (n * n).toInt<uint32_t>());
 }
 
+static_assert(is_trivially_copyable<ScaledNumber<uint32_t>>::value,
+              "trivially copyable");
+
 } // end namespace