Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][TableGen] Change NeonEmitter to use const Record * #110597

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 30, 2024

@jurahul jurahul force-pushed the const_record_clang_neon_emitter branch from 461d770 to 25ab164 Compare September 30, 2024 23:01
@jurahul jurahul marked this pull request as ready for review October 1, 2024 03:23
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 1, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 1, 2024

@llvm/pr-subscribers-clang

Author: Rahul Joshi (jurahul)

Changes

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089


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

2 Files Affected:

  • (modified) clang/utils/TableGen/NeonEmitter.cpp (+1-2)
  • (modified) clang/utils/TableGen/SveEmitter.cpp (+5-7)
diff --git a/clang/utils/TableGen/NeonEmitter.cpp b/clang/utils/TableGen/NeonEmitter.cpp
index 202573e2ac740e..d4b42360e7fd32 100644
--- a/clang/utils/TableGen/NeonEmitter.cpp
+++ b/clang/utils/TableGen/NeonEmitter.cpp
@@ -410,8 +410,7 @@ class Intrinsic {
     }
 
     int ArgIdx, Kind, TypeArgIdx;
-    std::vector<Record *> ImmCheckList = R->getValueAsListOfDefs("ImmChecks");
-    for (const auto *I : ImmCheckList) {
+    for (const Record *I : R->getValueAsListOfDefs("ImmChecks")) {
       unsigned EltSizeInBits = 0, VecSizeInBits = 0;
 
       ArgIdx = I->getValueAsInt("ImmArgIdx");
diff --git a/clang/utils/TableGen/SveEmitter.cpp b/clang/utils/TableGen/SveEmitter.cpp
index 2f9747e7de3de2..405a58e49e0dc9 100644
--- a/clang/utils/TableGen/SveEmitter.cpp
+++ b/clang/utils/TableGen/SveEmitter.cpp
@@ -1161,11 +1161,9 @@ void SVEEmitter::createIntrinsic(
   uint64_t Merge = R->getValueAsInt("Merge");
   StringRef MergeSuffix = R->getValueAsString("MergeSuffix");
   uint64_t MemEltType = R->getValueAsInt("MemEltType");
-  std::vector<Record*> FlagsList = R->getValueAsListOfDefs("Flags");
-  std::vector<Record*> ImmCheckList = R->getValueAsListOfDefs("ImmChecks");
 
   int64_t Flags = 0;
-  for (auto FlagRec : FlagsList)
+  for (auto FlagRec : R->getValueAsListOfConstDefs("Flags"))
     Flags |= FlagRec->getValueAsInt("Value");
 
   // Create a dummy TypeSpec for non-overloaded builtins.
@@ -1195,10 +1193,10 @@ void SVEEmitter::createIntrinsic(
   for (auto TS : TypeSpecs) {
     // Collate a list of range/option checks for the immediates.
     SmallVector<ImmCheck, 2> ImmChecks;
-    for (auto *R : ImmCheckList) {
-      int64_t ArgIdx = R->getValueAsInt("ImmArgIdx");
-      int64_t EltSizeArgIdx = R->getValueAsInt("TypeContextArgIdx");
-      int64_t Kind = R->getValueAsDef("Kind")->getValueAsInt("Value");
+    for (auto ImmR : R->getValueAsListOfConstDefs("ImmChecks")) {
+      int64_t ArgIdx = ImmR->getValueAsInt("ImmArgIdx");
+      int64_t EltSizeArgIdx = ImmR->getValueAsInt("TypeContextArgIdx");
+      int64_t Kind = ImmR->getValueAsDef("Kind")->getValueAsInt("Value");
       assert(ArgIdx >= 0 && Kind >= 0 &&
              "ImmArgIdx and Kind must be nonnegative");
 

clang/utils/TableGen/SveEmitter.cpp Outdated Show resolved Hide resolved
clang/utils/TableGen/SveEmitter.cpp Outdated Show resolved Hide resolved
@jurahul jurahul force-pushed the const_record_clang_neon_emitter branch from 7d9e44f to 8887de9 Compare October 1, 2024 16:50
Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jurahul
Copy link
Contributor Author

jurahul commented Oct 1, 2024

Thanks!

@jurahul jurahul merged commit e9dbdb2 into llvm:main Oct 1, 2024
6 of 7 checks passed
@jurahul jurahul deleted the const_record_clang_neon_emitter branch October 1, 2024 17:47
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants