Skip to content

[LLVM][NFC] Reduce copying of parameter in lambda #110299

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

Merged

Conversation

AmrDeveloper
Copy link
Member

@AmrDeveloper AmrDeveloper commented Sep 27, 2024

Reduce redundant copy parameter in lambda

Fixes #95642

@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Sep 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2024

@llvm/pr-subscribers-llvm-analysis

Author: Amr Hesham (AmrDeveloper)

Changes

Remove redundant copy parameter in lambda

Fixes #95642


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/VectorUtils.cpp (+5-6)
diff --git a/llvm/lib/Analysis/VectorUtils.cpp b/llvm/lib/Analysis/VectorUtils.cpp
index dbffbb8a5f81d9..2532889a36c17f 100644
--- a/llvm/lib/Analysis/VectorUtils.cpp
+++ b/llvm/lib/Analysis/VectorUtils.cpp
@@ -1414,7 +1414,7 @@ void InterleavedAccessInfo::analyzeInterleaving(
 
   auto InvalidateGroupIfMemberMayWrap = [&](InterleaveGroup<Instruction> *Group,
                                             int Index,
-                                            std::string FirstOrLast) -> bool {
+                                            StringRef FirstOrLast) -> bool {
     Instruction *Member = Group->getMember(Index);
     assert(Member && "Group member does not exist");
     Value *MemberPtr = getLoadStorePointerOperand(Member);
@@ -1455,11 +1455,10 @@ void InterleavedAccessInfo::analyzeInterleaving(
     // So we check only group member 0 (which is always guaranteed to exist),
     // and group member Factor - 1; If the latter doesn't exist we rely on
     // peeling (if it is a non-reversed access -- see Case 3).
-    if (InvalidateGroupIfMemberMayWrap(Group, 0, std::string("first")))
+    if (InvalidateGroupIfMemberMayWrap(Group, 0, "first"))
       continue;
     if (Group->getMember(Group->getFactor() - 1))
-      InvalidateGroupIfMemberMayWrap(Group, Group->getFactor() - 1,
-                                     std::string("last"));
+      InvalidateGroupIfMemberMayWrap(Group, Group->getFactor() - 1, "last");
     else {
       // Case 3: A non-reversed interleaved load group with gaps: We need
       // to execute at least one scalar epilogue iteration. This will ensure
@@ -1503,11 +1502,11 @@ void InterleavedAccessInfo::analyzeInterleaving(
     // and the last group member. Case 3 (scalar epilog) is not relevant for
     // stores with gaps, which are implemented with masked-store (rather than
     // speculative access, as in loads).
-    if (InvalidateGroupIfMemberMayWrap(Group, 0, std::string("first")))
+    if (InvalidateGroupIfMemberMayWrap(Group, 0, "first"))
       continue;
     for (int Index = Group->getFactor() - 1; Index > 0; Index--)
       if (Group->getMember(Index)) {
-        InvalidateGroupIfMemberMayWrap(Group, Index, std::string("last"));
+        InvalidateGroupIfMemberMayWrap(Group, Index, "last");
         break;
       }
   }

@AmrDeveloper AmrDeveloper force-pushed the code_quality_analysis_pointless_str_copy branch 2 times, most recently from e9fbf5e to 1e6d708 Compare September 29, 2024 12:58
Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

Since the text is only being used in a debug message, it could be const char*. Then you're only passing a pointer as opposed to StringRef that's pointer + size.

Also please update the PR title to reflect what you choose to do.

This is reducing copies in a sense, but it's not removing it entirely.

@AmrDeveloper AmrDeveloper force-pushed the code_quality_analysis_pointless_str_copy branch from 1e6d708 to b9cc43b Compare October 15, 2024 16:22
@AmrDeveloper AmrDeveloper changed the title [LLVM][NFC] Remove redundant copy parameter in lambda [LLVM][NFC] Reduce redundant copy parameter in lambda Oct 15, 2024
@AmrDeveloper
Copy link
Member Author

Since the text is only being used in a debug message, it could be const char*. Then you're only passing a pointer as opposed to StringRef that's pointer + size.

Also please update the PR title to reflect what you choose to do.

This is reducing copies in a sense, but it's not removing it entirely.

Thank you, Done

@DavidSpickett DavidSpickett changed the title [LLVM][NFC] Reduce redundant copy parameter in lambda [LLVM][NFC] Reduce copying of parameter in lambda Oct 16, 2024
@DavidSpickett DavidSpickett merged commit 4ba1800 into llvm:main Oct 16, 2024
8 checks passed
@DavidSpickett
Copy link
Collaborator

Thanks for the improvements!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

llvm/lib/Analysis/VectorUtils.cpp:1354:: Pointless string copy ?
3 participants