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

[NFC] Use references to avoid copying #110462

Closed
wants to merge 8 commits into from
Closed

Conversation

pratyay-p
Copy link
Contributor

Modifying some uses of auto to auto& to avoid unnecessary copying.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 30, 2024

@llvm/pr-subscribers-tablegen

Author: Pratyay Pande (pratyay-p)

Changes

Modifying some uses of auto to auto& to avoid unnecessary copying.


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

1 Files Affected:

  • (modified) llvm/utils/TableGen/IntrinsicEmitter.cpp (+4-4)
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index efa067e60de439..469d0bbf3ae4b6 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -229,8 +229,8 @@ struct IntrinsicTargetInfo {
 };
 static constexpr IntrinsicTargetInfo TargetInfos[] = {
 )";
-  for (const auto [Name, Offset, Count] : Ints.Targets)
-    OS << formatv("  {{\"{}\", {}, {}},\n", Name, Offset, Count);
+  for (const auto &Target : Ints.Targets)
+    OS << formatv("  {{\"{}\", {}, {}},\n", Target.Name, Target.Offset, Target.Count);
   OS << R"(};
 #endif
 
@@ -255,7 +255,7 @@ void IntrinsicEmitter::EmitIntrinsicToOverloadTable(
 static constexpr uint8_t OTable[] = {
   0
   )";
-  for (auto [I, Int] : enumerate(Ints)) {
+  for (const auto& [I, Int] : enumerate(Ints)) {
     // Add one to the index so we emit a null bit for the invalid #0 intrinsic.
     size_t Idx = I + 1;
 
@@ -345,7 +345,7 @@ static constexpr {} IIT_Table[] = {{
                 FixedEncodingTypeName);
 
   unsigned MaxOffset = 0;
-  for (auto [Idx, FixedEncoding, Int] : enumerate(FixedEncodings, Ints)) {
+  for (const auto& [Idx, FixedEncoding, Int] : enumerate(FixedEncodings, Ints)) {
     if ((Idx & 7) == 7)
       OS << "\n  ";
 

@pratyay-p
Copy link
Contributor Author

@KanRobert , @phoebewang , @MalaySanghi can you please review?

Copy link

github-actions bot commented Sep 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -256,7 +256,7 @@ void IntrinsicEmitter::EmitIntrinsicToOverloadTable(
static constexpr uint8_t OTable[] = {
0
)";
for (auto [I, Int] : enumerate(Ints)) {
for (const auto [I, Int] : enumerate(Ints)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given it is just a copy, do we still need const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed const

@@ -346,7 +346,7 @@ static constexpr {} IIT_Table[] = {{
FixedEncodingTypeName);

unsigned MaxOffset = 0;
for (auto [Idx, FixedEncoding, Int] : enumerate(FixedEncodings, Ints)) {
for (const auto [Idx, FixedEncoding, Int] : enumerate(FixedEncodings, Ints)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know why leaving const here. I had a quick search. const auto [ is much smaller used in LLVM code base.

$ grep -r 'const auto \[' llvm/| wc
     37     275    3760
$ grep -r 'auto \[' llvm/| wc
    817    5871   82133

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. It seems that we wont need to change anything in this PR. Closing this.

@pratyay-p pratyay-p closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants