[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