[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