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

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D92509#2453504 <https://reviews.llvm.org/D92509#2453504>, @rnk wrote:

> 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.

I missed this email yesterday, but your solution SGTM.

It looks like you did not revert 65c5c9f92ec514ae41c8ea407d1c885737d699ec <https://reviews.llvm.org/rG65c5c9f92ec514ae41c8ea407d1c885737d699ec> (ADT: Rely on std::aligned_union_t for math in AlignedCharArrayUnion, NFC), which changed it to rely on the math in `std::aligned_union_t`:

  template <typename T, typename... Ts> struct AlignedCharArrayUnion {
    using AlignedUnion = std::aligned_union_t<1, T, Ts...>;
    alignas(alignof(AlignedUnion)) char buffer[sizeof(AlignedUnion)];
  };

Should that be reverted too, or if not, why does `alignof(std::aligned_union_t<...>)` give us the right answer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92509