Skip to content

Conversation

@jph-13
Copy link
Contributor

@jph-13 jph-13 commented Feb 28, 2025

Theses consts in ASMTargetParser were causing unnecessary global initialization fuctions.
_GLOBAL__sub_I_ARMTargetParser.cpp
_GLOBAL__sub_I_Triple.cpp

Both functions init the same consts.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-ir

Author: JP Hafer (jph-13)

Changes

Theses consts in ASMTargetParser were causing unnecessary global initialization fuctions.
_GLOBAL__sub_I_ARMTargetParser.cpp
_GLOBAL__sub_I_Triple.cpp

Both functions init the same consts.


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

2 Files Affected:

  • (modified) llvm/include/llvm/TargetParser/ARMTargetParser.h (+5-5)
  • (modified) llvm/lib/IR/AsmWriter.cpp (-1)
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParser.h b/llvm/include/llvm/TargetParser/ARMTargetParser.h
index 2b0ef76a6b51f..5dbcfd3d2d693 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -77,7 +77,7 @@ struct ExtName {
   StringRef NegFeature;
 };
 
-const ExtName ARCHExtNames[] = {
+constexpr ExtName ARCHExtNames[] = {
 #define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE)                       \
   {NAME, ID, FEATURE, NEGFEATURE},
 #include "ARMTargetParser.def"
@@ -85,7 +85,7 @@ const ExtName ARCHExtNames[] = {
 
 // List of HWDiv names (use getHWDivSynonym) and which architectural
 // features they correspond to (use getHWDivFeatures).
-const struct {
+constexpr struct {
   StringRef Name;
   uint64_t ID;
 } HWDivNames[] = {
@@ -112,7 +112,7 @@ struct CpuNames {
   uint64_t DefaultExtensions;
 };
 
-const CpuNames CPUNames[] = {
+constexpr CpuNames CPUNames[] = {
 #define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT)           \
   {NAME, ARM::ArchKind::ID, IS_DEFAULT, DEFAULT_EXT},
 #include "ARMTargetParser.def"
@@ -173,7 +173,7 @@ struct FPUName {
   FPURestriction Restriction;
 };
 
-static const FPUName FPUNames[] = {
+static constexpr FPUName FPUNames[] = {
 #define ARM_FPU(NAME, KIND, VERSION, NEON_SUPPORT, RESTRICTION)                \
   {NAME, KIND, VERSION, NEON_SUPPORT, RESTRICTION},
 #include "llvm/TargetParser/ARMTargetParser.def"
@@ -199,7 +199,7 @@ struct ArchNames {
   StringRef getSubArch() const { return ArchFeature.substr(1); }
 };
 
-static const ArchNames ARMArchNames[] = {
+static constexpr ArchNames ARMArchNames[] = {
 #define ARM_ARCH(NAME, ID, CPU_ATTR, ARCH_FEATURE, ARCH_ATTR, ARCH_FPU,        \
                  ARCH_BASE_EXT)                                                \
   {NAME,          CPU_ATTR,     ARCH_FEATURE, ARCH_FPU,                        \
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index a52c4d88ac836..306186d200e26 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2117,7 +2117,6 @@ static void writeDIGenericSubrange(raw_ostream &Out, const DIGenericSubrange *N,
   };
 
   auto GetConstant = [&](Metadata *Bound) -> int64_t {
-    assert(IsConstant(Bound) && "Expected constant");
     auto *BE = dyn_cast_or_null<DIExpression>(Bound);
     return static_cast<int64_t>(BE->getElement(1));
   };

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this unrelated to the constexpr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Sorry for the confusion. That was supposed to be on a different PR. It was just another thing noticed.

@jph-13 jph-13 force-pushed the remove_unneeded_global_inits branch from 8359b66 to 57deb17 Compare February 28, 2025 16:03
@jph-13
Copy link
Contributor Author

jph-13 commented Feb 28, 2025

I completely messed this up trying to clean up the merge commits. I will create a fresh one. Sorry for the inconvenience.

@jph-13 jph-13 closed this Feb 28, 2025
@jph-13 jph-13 deleted the remove_unneeded_global_inits branch February 28, 2025 20:21
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.

4 participants