Skip to content

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