[PATCH] D138792: [AArch64] Improve TargetParser API

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added inline comments.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:115-118
+  ArchInfo(const ArchInfo &) = delete;
+  ArchInfo(const ArchInfo &&) = delete;
+  ArchInfo &operator=(const ArchInfo &rhs) = delete;
+  ArchInfo &&operator=(const ArchInfo &&rhs) = delete;
----------------
If this is really non-copyable add a (constexpr) constructor. non-copyable and aggregate initialization doesn't mix. C++20 doesn't allow it, C++17 accidentally has a lot of loopholes around the non-copyability if you do aggregate initialization.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:154-160
+// Create ArchInfo structs named <ID>
+#define AARCH64_ARCH(MAJOR, MINOR, PROFILE, NAME, ID, ARCH_FEATURE,            \
+                     ARCH_BASE_EXT)                                            \
+  inline constexpr ArchInfo ID = {VersionTuple{MAJOR, MINOR}, PROFILE, NAME,   \
+                                  ARCH_FEATURE, ARCH_BASE_EXT};
+#include "AArch64TargetParser.def"
+#undef AARCH64_ARCH
----------------
Is there a good reason for these to be defined in the header? This was wrong before and now works because of inline constexpr, but it's still wasting a bunch of compile time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138792