[llvm] r239872 - Fix alignment issues in LLVM.

Justin Bogner mail at justinbogner.com
Tue Jun 16 20:07:57 PDT 2015


James Y Knight <jyknight at google.com> writes:
> Author: jyknight
> Date: Tue Jun 16 20:21:20 2015
> New Revision: 239872
>
> URL: http://llvm.org/viewvc/llvm-project?rev=239872&view=rev
> Log:
> Fix alignment issues in LLVM.
>
> Adds static_asserts to ensure alignment of concatenated objects is
> correct, and fixes them where they are not.
>
> Also changes the definition of AlignOf to use constexpr, except on
> MSVC, to avoid enum comparison warnings from GCC.
>
> (There's not too much of this in llvm itself, most of the fun is in
> clang).
>
> This seems to make LLVM actually work without Bus Error on 32bit
> sparc.
>
> Differential Revision: http://reviews.llvm.org/D10271
>
> Modified:
>     llvm/trunk/include/llvm/IR/DerivedTypes.h
>     llvm/trunk/include/llvm/IR/User.h
>     llvm/trunk/include/llvm/Support/AlignOf.h
>     llvm/trunk/lib/IR/AttributeImpl.h
>     llvm/trunk/lib/IR/Metadata.cpp
>     llvm/trunk/lib/IR/User.cpp
>
> Modified: llvm/trunk/include/llvm/IR/DerivedTypes.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DerivedTypes.h?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DerivedTypes.h (original)
> +++ llvm/trunk/include/llvm/IR/DerivedTypes.h Tue Jun 16 20:21:20 2015
> @@ -140,7 +140,8 @@ public:
>      return T->getTypeID() == FunctionTyID;
>    }
>  };
> -
> +static_assert(AlignOf<FunctionType>::Alignment >= AlignOf<Type *>::Alignment,
> +              "Alignment sufficient for objects appended to FunctionType");

I find these assert messages a bit hard to parse. It's better to have
the message be a sentence describing what's wrong, rather than a
statement about what's expected.

>  
>  /// CompositeType - Common super class of ArrayType, StructType, PointerType
>  /// and VectorType.
>
> Modified: llvm/trunk/include/llvm/IR/User.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/User.h?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/User.h (original)
> +++ llvm/trunk/include/llvm/IR/User.h Tue Jun 16 20:21:20 2015
> @@ -22,6 +22,7 @@
>  #include "llvm/ADT/iterator.h"
>  #include "llvm/ADT/iterator_range.h"
>  #include "llvm/IR/Value.h"
> +#include "llvm/Support/AlignOf.h"
>  #include "llvm/Support/ErrorHandling.h"
>  
>  namespace llvm {
> @@ -226,6 +227,11 @@ public:
>      return isa<Instruction>(V) || isa<Constant>(V);
>    }
>  };
> +// Either Use objects, or a Use pointer can be prepended to User.
> +static_assert(AlignOf<Use>::Alignment >= AlignOf<User>::Alignment,
> +              "Alignment sufficient after objects prepended to User");
> +static_assert(AlignOf<Use *>::Alignment >= AlignOf<User>::Alignment,
> +              "Alignment sufficient after objects prepended to User");
>  
>  template<> struct simplify_type<User::op_iterator> {
>    typedef Value* SimpleType;
>
> Modified: llvm/trunk/include/llvm/Support/AlignOf.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/AlignOf.h?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/AlignOf.h (original)
> +++ llvm/trunk/include/llvm/Support/AlignOf.h Tue Jun 16 20:21:20 2015
> @@ -44,9 +44,18 @@ private:
>  ///  compile-time constant (e.g., for template instantiation).
>  template <typename T>
>  struct AlignOf {
> +#ifndef _MSC_VER
> +  // Avoid warnings from GCC like:
> +  //   comparison between 'enum llvm::AlignOf<X>::<anonymous>' and 'enum
> +  //   llvm::AlignOf<Y>::<anonymous>' [-Wenum-compare]
> +  // by using constexpr instead of enum.
> +  // (except on MSVC, since it doesn't support constexpr yet).
> +  static constexpr unsigned Alignment =
> +      static_cast<unsigned int>(sizeof(AlignmentCalcImpl<T>) - sizeof(T));
> +#else
>    enum { Alignment =
>           static_cast<unsigned int>(sizeof(AlignmentCalcImpl<T>) - sizeof(T)) };
> -
> +#endif
>    enum { Alignment_GreaterEqual_2Bytes = Alignment >= 2 ? 1 : 0 };
>    enum { Alignment_GreaterEqual_4Bytes = Alignment >= 4 ? 1 : 0 };
>    enum { Alignment_GreaterEqual_8Bytes = Alignment >= 8 ? 1 : 0 };
> @@ -58,6 +67,10 @@ struct AlignOf {
>    enum { Alignment_LessEqual_16Bytes = Alignment <= 16 ? 1 : 0 };
>  };
>  
> +#ifndef _MSC_VER
> +template <typename T> constexpr unsigned AlignOf<T>::Alignment;
> +#endif
> +
>  /// alignOf - A templated function that returns the minimum alignment of
>  ///  of a type.  This provides no extra functionality beyond the AlignOf
>  ///  class besides some cosmetic cleanliness.  Example usage:
>
> Modified: llvm/trunk/lib/IR/AttributeImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/AttributeImpl.h?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/AttributeImpl.h (original)
> +++ llvm/trunk/lib/IR/AttributeImpl.h Tue Jun 16 20:21:20 2015
> @@ -181,6 +181,9 @@ public:
>        AttrList[I].Profile(ID);
>    }
>  };
> +static_assert(AlignOf<AttributeSetNode>::Alignment >=
> +                  AlignOf<Attribute>::Alignment,
> +              "Alignment sufficient for objects appended to AttributeSetNode");
>  
>  //===----------------------------------------------------------------------===//
>  /// \class
> @@ -189,9 +192,11 @@ public:
>  class AttributeSetImpl : public FoldingSetNode {
>    friend class AttributeSet;
>  
> -  LLVMContext &Context;
> -
> +public:
>    typedef std::pair<unsigned, AttributeSetNode*> IndexAttrPair;
> +
> +private:
> +  LLVMContext &Context;
>    unsigned NumAttrs; ///< Number of entries in this set.
>  
>    /// \brief Return a pointer to the IndexAttrPair for the specified slot.
> @@ -206,6 +211,7 @@ public:
>    AttributeSetImpl(LLVMContext &C,
>                     ArrayRef<std::pair<unsigned, AttributeSetNode *> > Attrs)
>        : Context(C), NumAttrs(Attrs.size()) {
> +
>  #ifndef NDEBUG
>      if (Attrs.size() >= 2) {
>        for (const std::pair<unsigned, AttributeSetNode *> *i = Attrs.begin() + 1,
> @@ -267,6 +273,9 @@ public:
>  
>    void dump() const;
>  };
> +static_assert(AlignOf<AttributeSetImpl>::Alignment >=
> +                  AlignOf<AttributeSetImpl::IndexAttrPair>::Alignment,
> +              "Alignment sufficient for objects appended to AttributeSetImpl");
>  
>  } // end llvm namespace
>  
>
> Modified: llvm/trunk/lib/IR/Metadata.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Metadata.cpp (original)
> +++ llvm/trunk/lib/IR/Metadata.cpp Tue Jun 16 20:21:20 2015
> @@ -381,20 +381,35 @@ StringRef MDString::getString() const {
>  // MDNode implementation.
>  //
>  
> +// Assert that the MDNode types will not be unaligned by the objects
> +// prepended to them.
> +#define HANDLE_MDNODE_LEAF(CLASS)                                              \
> +  static_assert(llvm::AlignOf<uint64_t>::Alignment >=                          \
> +                    llvm::AlignOf<CLASS>::Alignment,                           \
> +                "Alignment sufficient after objects prepended to " #CLASS);
> +#include "llvm/IR/Metadata.def"
> +
>  void *MDNode::operator new(size_t Size, unsigned NumOps) {
> -  void *Ptr = ::operator new(Size + NumOps * sizeof(MDOperand));
> +  size_t OpSize = NumOps * sizeof(MDOperand);
> +  // uint64_t is the most aligned type we need support (ensured by static_assert
> +  // above)
> +  OpSize = RoundUpToAlignment(OpSize, llvm::alignOf<uint64_t>());
> +  void *Ptr = reinterpret_cast<char *>(::operator new(OpSize + Size)) + OpSize;
>    MDOperand *O = static_cast<MDOperand *>(Ptr);
> -  for (MDOperand *E = O + NumOps; O != E; ++O)
> -    (void)new (O) MDOperand;
> -  return O;
> +  for (MDOperand *E = O - NumOps; O != E; --O)
> +    (void)new (O - 1) MDOperand;
> +  return Ptr;
>  }
>  
>  void MDNode::operator delete(void *Mem) {
>    MDNode *N = static_cast<MDNode *>(Mem);
> +  size_t OpSize = N->NumOperands * sizeof(MDOperand);
> +  OpSize = RoundUpToAlignment(OpSize, llvm::alignOf<uint64_t>());
> +
>    MDOperand *O = static_cast<MDOperand *>(Mem);
>    for (MDOperand *E = O - N->NumOperands; O != E; --O)
>      (O - 1)->~MDOperand();
> -  ::operator delete(O);
> +  ::operator delete(reinterpret_cast<char *>(Mem) - OpSize);
>  }
>  
>  MDNode::MDNode(LLVMContext &Context, unsigned ID, StorageType Storage,
>
> Modified: llvm/trunk/lib/IR/User.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/User.cpp?rev=239872&r1=239871&r2=239872&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/User.cpp (original)
> +++ llvm/trunk/lib/IR/User.cpp Tue Jun 16 20:21:20 2015
> @@ -42,6 +42,12 @@ void User::replaceUsesOfWith(Value *From
>  
>  void User::allocHungoffUses(unsigned N, bool IsPhi) {
>    assert(HasHungOffUses && "alloc must have hung off uses");
> +
> +  static_assert(AlignOf<Use>::Alignment >= AlignOf<Use::UserRef>::Alignment,
> +                "Alignment sufficient for hung-off-uses pieces");
> +  static_assert(AlignOf<Use::UserRef>::Alignment >= AlignOf<BasicBlock *>::Alignment,
> +                "Alignment sufficient for hung-off-uses pieces");
> +
>    // Allocate the array of Uses, followed by a pointer (with bottom bit set) to
>    // the User.
>    size_t size = N * sizeof(Use) + sizeof(Use::UserRef);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list