[PATCH] Fix alignment issues in LLVM.

Reid Kleckner rnk at google.com
Tue Jun 16 09:32:49 PDT 2015


BTW, Duncan recently rewrote this so I'll add him.


================
Comment at: include/llvm/Support/AlignOf.h:56-57
@@ -47,2 +55,4 @@
+#else
   enum { Alignment =
          static_cast<unsigned int>(sizeof(AlignmentCalcImpl<T>) - sizeof(T)) };
+#endif
----------------
jyknight wrote:
> majnemer wrote:
> > Why not just do `enum : unsigned { Alignment = ... ;` ?
> I hadn't tried until you suggested, but the answer is: because GCC still warns about it.
A static constexpr int still sometimes requires an out of class definition if you accidentally take the address of the global by passing it to a reference-taking function like std::max, which is a pretty common operation for alignments.

I'd rather add casts in the assertions or disable gcc's warning. Seem reasonable?

================
Comment at: lib/IR/Metadata.cpp:393-396
@@ -384,2 +392,6 @@
 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)
+  size_t AlignOffset = OffsetToAlignment(OpSize, llvm::alignOf<uint64_t>());
+  void *Ptr =
----------------
I feel like a more intuitive way to phrase this computation is to overallocate like so:
  size_t OpSize = NumOps * sizeof(MDOperand);
  OpSize = RoundUpToAlignment(OpSize, llvm::alignOf<uint64_t>());

The placement new operations can use the same negative indexing approach as operator delete, and we never have to consider AlignOffset as a separate quantity.

================
Comment at: lib/IR/Metadata.cpp:411-412
@@ -395,1 +410,4 @@
+
+  MDOperand *O =
+      reinterpret_cast<MDOperand *>(reinterpret_cast<uintptr_t>(Mem));
   for (MDOperand *E = O - N->NumOperands; O != E; --O)
----------------
What was wrong with the old code here? A strict aliasing warning?

http://reviews.llvm.org/D10271

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list