Skip to content

Conversation

@hahnjo
Copy link
Member

@hahnjo hahnjo commented Feb 16, 2025

All std::aligned_* are deprecated in C++23. Implement the replacement suggested in P1413R3 using alignas and std::max.

All std::aligned_* are deprecated in C++23. Implement the replacement
suggested in P1413R3 using alignas and std::max.
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2025

@llvm/pr-subscribers-llvm-support

Author: Jonas Hahnfeld (hahnjo)

Changes

All std::aligned_* are deprecated in C++23. Implement the replacement suggested in P1413R3 using alignas and std::max.


Full diff: https://github.com/llvm/llvm-project/pull/127417.diff

1 Files Affected:

  • (modified) llvm/include/llvm/Support/AlignOf.h (+2-8)
diff --git a/llvm/include/llvm/Support/AlignOf.h b/llvm/include/llvm/Support/AlignOf.h
index f586d7f182aab..635bb8b101ef9 100644
--- a/llvm/include/llvm/Support/AlignOf.h
+++ b/llvm/include/llvm/Support/AlignOf.h
@@ -13,20 +13,14 @@
 #ifndef LLVM_SUPPORT_ALIGNOF_H
 #define LLVM_SUPPORT_ALIGNOF_H
 
-#include <type_traits>
+#include <algorithm>
 
 namespace llvm {
 
 /// A suitably aligned and sized character array member which can hold elements
 /// of any type.
-///
-/// This template is equivalent to std::aligned_union_t<1, ...>, but we cannot
-/// use it due to a bug in the MSVC x86 compiler:
-/// https://github.com/microsoft/STL/issues/1533
-/// Using `alignas` here works around the bug.
 template <typename T, typename... Ts> struct AlignedCharArrayUnion {
-  using AlignedUnion = std::aligned_union_t<1, T, Ts...>;
-  alignas(alignof(AlignedUnion)) char buffer[sizeof(AlignedUnion)];
+  alignas(T) alignas(Ts...) char buffer[std::max({sizeof(T), sizeof(Ts)...})];
 };
 
 } // end namespace llvm

@hahnjo hahnjo requested a review from dwblaikie February 25, 2025 09:59
@hahnjo hahnjo merged commit f1025e6 into llvm:main Feb 25, 2025
10 checks passed
@hahnjo hahnjo deleted the AlignOf branch February 25, 2025 20:00
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 25, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-sles-build-only running on rocm-worker-hw-04-sles while building llvm at step 5 "compile-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/17695

Here is the relevant piece of the build log for the reference
Step 5 (compile-openmp) failure: build (failure)
...
     for (const auto &[SC, _] : SuperClasses) {
                            ^
3.073 [6880/32/195] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/Timer.cpp.o
3.083 [6879/32/196] Building CXX object lib/Option/CMakeFiles/LLVMOption.dir/Arg.cpp.o
3.091 [6878/32/197] Building CXX object lib/DebugInfo/CodeView/CMakeFiles/LLVMDebugInfoCodeView.dir/Line.cpp.o
3.093 [6877/32/198] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/TimeProfiler.cpp.o
3.113 [6876/32/199] Building CXX object lib/DebugInfo/CodeView/CMakeFiles/LLVMDebugInfoCodeView.dir/RecordName.cpp.o
3.114 [6875/32/200] Building CXX object lib/DebugInfo/CodeView/CMakeFiles/LLVMDebugInfoCodeView.dir/RecordSerialization.cpp.o
3.131 [6874/32/201] Building CXX object lib/DebugInfo/CodeView/CMakeFiles/LLVMDebugInfoCodeView.dir/SimpleTypeSerializer.cpp.o
3.149 [6873/32/202] Building CXX object utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/CodeGenIntrinsics.cpp.o
FAILED: utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/CodeGenIntrinsics.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iutils/TableGen/Basic -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic -Iinclude -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++1z -MD -MT utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/CodeGenIntrinsics.cpp.o -MF utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/CodeGenIntrinsics.cpp.o.d -o utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/CodeGenIntrinsics.cpp.o -c /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp:19:0:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h: In member function ‘bool llvm::Record::isSubClassOf(const llvm::Record*) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h:1797:28: warning: unused variable ‘_’ [-Wunused-variable]
     for (const auto &[SC, _] : SuperClasses)
                            ^
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h: In member function ‘bool llvm::Record::isSubClassOf(llvm::StringRef) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h:1804:28: warning: unused variable ‘_’ [-Wunused-variable]
     for (const auto &[SC, _] : SuperClasses) {
                            ^
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:19:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h:18,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp:13:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/Support/AlignOf.h: In instantiation of ‘struct llvm::AlignedCharArrayUnion<llvm::detail::DenseSetPair<llvm::StringRef> [4]>’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:1049:53:   required from ‘void llvm::SmallDenseMap<KeyT, ValueT, InlineBuckets, KeyInfoT, BucketT>::grow(unsigned int) [with KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; unsigned int InlineBuckets = 4; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef, void>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:536:33:   required from ‘void llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::grow(unsigned int) [with DerivedT = llvm::SmallDenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, 4, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseSetPair<llvm::StringRef> >; KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef, void>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:576:13:   required from ‘BucketT* llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::InsertIntoBucketImpl(const LookupKeyT&, BucketT*) [with LookupKeyT = llvm::StringRef; DerivedT = llvm::SmallDenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, 4, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseSetPair<llvm::StringRef> >; KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef, void>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:543:37:   required from ‘BucketT* llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::InsertIntoBucket(BucketT*, KeyArg&&, ValueArgs&& ...) [with KeyArg = const llvm::StringRef&; ValueArgs = {llvm::detail::DenseSetEmpty&}; DerivedT = llvm::SmallDenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, 4, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseSetPair<llvm::StringRef> >; KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef, void>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseMap.h:262:33:   required from ‘std::pair<llvm::DenseMapIterator<KeyT, ValueT, KeyInfoT, BucketT>, bool> llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::try_emplace(const KeyT&, Ts&& ...) [with Ts = {llvm::detail::DenseSetEmpty&}; DerivedT = llvm::SmallDenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, 4, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseSetPair<llvm::StringRef> >; KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef, void>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/DenseSet.h:215:39:   required from ‘std::pair<llvm::detail::DenseSetImpl<ValueT, MapTy, ValueInfoT>::Iterator, bool> llvm::detail::DenseSetImpl<ValueT, MapTy, ValueInfoT>::insert(const ValueT&) [with ValueT = llvm::StringRef; MapTy = llvm::SmallDenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, 4, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseSetPair<llvm::StringRef> >; ValueInfoT = llvm::DenseMapInfo<llvm::StringRef, void>]’
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp:111:35:   required from here
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/Support/AlignOf.h:23:77: internal compiler error: Segmentation fault
   alignas(T) alignas(Ts...) char buffer[std::max({sizeof(T), sizeof(Ts)...})];
                                                                             ^
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.opensuse.org/> for instructions.
3.173 [6873/31/203] Building CXX object lib/Option/CMakeFiles/LLVMOption.dir/Option.cpp.o
3.184 [6873/30/204] Building CXX object utils/TableGen/Basic/CMakeFiles/obj.LLVMTableGenBasic.dir/Attributes.cpp.o
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/TableGen/Basic/Attributes.cpp:10:0:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h: In member function ‘bool llvm::Record::isSubClassOf(const llvm::Record*) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h:1797:28: warning: unused variable ‘_’ [-Wunused-variable]
     for (const auto &[SC, _] : SuperClasses)
                            ^
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h: In member function ‘bool llvm::Record::isSubClassOf(llvm::StringRef) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/TableGen/Record.h:1804:28: warning: unused variable ‘_’ [-Wunused-variable]
     for (const auto &[SC, _] : SuperClasses) {
                            ^

@jplehr
Copy link
Contributor

jplehr commented Feb 25, 2025

Hi,
Can this be fixed forward for GCC 7.5 on SLES or should we revert in the meantime?
Thanks!

@hahnjo
Copy link
Member Author

hahnjo commented Feb 25, 2025

@jplehr I played a bit with Compiler Explorer and it seems the problem is alignas(Ts...), I couldn't get it to work with any parameter pack. But static constexpr std::size_t Align = std::max({alignof(T), alignof(Ts)...}); seems to work (https://godbolt.org/z/Mf3hzezPa) and should be fine because it's all power-of-two's. Do you have easy access to the bot / a similar machine? If not, I can commit and we revert if it still doesn't work...

@jplehr
Copy link
Contributor

jplehr commented Feb 25, 2025

I don't mind if you land and we see if it fixes the issue and we revert in case it does not.

@hahnjo
Copy link
Member Author

hahnjo commented Feb 25, 2025

Pushed 6a5dd04, fingers crossed 🤞

@jplehr
Copy link
Contributor

jplehr commented Feb 25, 2025

Looks good. Thank you for that quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants