[PATCH] D92509: ADT: Remove redundant `alignas` from IntervalMap, NFC

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 16:55:41 PST 2020


rnk added a comment.

The problem is that MSVC x86 doesn't actually guarantee the 8 byte alignment that we want:
https://github.com/microsoft/STL/issues/1533
On x86, the Visual C++ compiler only assumes 4 byte stack alignment. The compiler doesn't realign the stack for types that do not explicitly use alignas. So, regular double and uint64_t local variables are not actually 8 byte aligned. The STL implements std::aligned* without using `alignas`, so the compiler doesn't realign the stack for us.

We can either:

1. Go back to using our own type AlignedCharArrayUnion that works reliably
2. Work around the issue with redundant `alignas` attributes on all the variables that matter

I will also note that MSVC's std::aligned_storage doesn't support alignments of 16 or higher unless you pass -D_ENABLE_EXTENDED_ALIGNED_STORAGE, but AlignedCharArrayUnion would support such alignments.

I think I lean towards option #1, and I can get to it with straight reverts, so I think I'm going to go ahead and do that for now. I'm OK with option 2, if people like the std:: spelling of this better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92509/new/

https://reviews.llvm.org/D92509



More information about the llvm-commits mailing list