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

[flang] Resolve "possible performance problem" issue spam #79769

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

klausler
Copy link
Contributor

Four "issues" on GitHub report possible performance problems, likely detected by static analysis. None of them would ever make a measureable difference in compilation time, but I'm resolving them to clean up the open issues list.

Fixes #79703, .../79705, .../79706, & .../79707.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 28, 2024

@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-flang-codegen
@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-fir-hlfir

Author: Peter Klausler (klausler)

Changes

Four "issues" on GitHub report possible performance problems, likely detected by static analysis. None of them would ever make a measureable difference in compilation time, but I'm resolving them to clean up the open issues list.

Fixes #79703, .../79705, .../79706, & .../79707.


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

5 Files Affected:

  • (modified) flang/lib/Lower/OpenACC.cpp (+1-1)
  • (modified) flang/lib/Optimizer/CodeGen/Target.cpp (+1-1)
  • (modified) flang/lib/Parser/preprocessor.cpp (+1-1)
  • (modified) flang/lib/Parser/preprocessor.h (+1-1)
  • (modified) flang/lib/Semantics/check-directive-structure.h (+2-2)
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index ecfdaa5be993584..72a0c195d96bb1e 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -3208,7 +3208,7 @@ static void createDeclareGlobalOp(mlir::OpBuilder &modBuilder,
                                   fir::FirOpBuilder &builder,
                                   mlir::Location loc, fir::GlobalOp globalOp,
                                   mlir::acc::DataClause clause,
-                                  const std::string declareGlobalName,
+                                  const std::string &declareGlobalName,
                                   bool implicit, std::stringstream &asFortran) {
   GlobalOp declareGlobalOp =
       modBuilder.create<GlobalOp>(loc, declareGlobalName);
diff --git a/flang/lib/Optimizer/CodeGen/Target.cpp b/flang/lib/Optimizer/CodeGen/Target.cpp
index a4df0b09177ab75..7d0c6d7e7906bef 100644
--- a/flang/lib/Optimizer/CodeGen/Target.cpp
+++ b/flang/lib/Optimizer/CodeGen/Target.cpp
@@ -47,7 +47,7 @@ static const llvm::fltSemantics &floatToSemantics(const KindMapping &kindMap,
 }
 
 static void typeTodo(const llvm::fltSemantics *sem, mlir::Location loc,
-                     std::string context) {
+                     const std::string &context) {
   if (sem == &llvm::APFloat::IEEEhalf()) {
     TODO(loc, "COMPLEX(KIND=2): for " + context + " type");
   } else if (sem == &llvm::APFloat::BFloat()) {
diff --git a/flang/lib/Parser/preprocessor.cpp b/flang/lib/Parser/preprocessor.cpp
index 8c993e7ced0e8c8..75e4c469774fe0a 100644
--- a/flang/lib/Parser/preprocessor.cpp
+++ b/flang/lib/Parser/preprocessor.cpp
@@ -252,7 +252,7 @@ void Preprocessor::DefineStandardMacros() {
   Define("__LINE__"s, "__LINE__"s);
 }
 
-void Preprocessor::Define(std::string macro, std::string value) {
+void Preprocessor::Define(const std::string &macro, const std::string &value) {
   definitions_.emplace(SaveTokenAsName(macro), Definition{value, allSources_});
 }
 
diff --git a/flang/lib/Parser/preprocessor.h b/flang/lib/Parser/preprocessor.h
index 3b456364944c3d6..b61f1577727beb7 100644
--- a/flang/lib/Parser/preprocessor.h
+++ b/flang/lib/Parser/preprocessor.h
@@ -70,7 +70,7 @@ class Preprocessor {
   AllSources &allSources() { return allSources_; }
 
   void DefineStandardMacros();
-  void Define(std::string macro, std::string value);
+  void Define(const std::string &macro, const std::string &value);
   void Undefine(std::string macro);
   bool IsNameDefined(const CharBlock &);
   bool IsFunctionLikeDefinition(const CharBlock &);
diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index 829405f99d64c0a..97e13c59ac4167a 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -176,8 +176,8 @@ template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
 class DirectiveStructureChecker : public virtual BaseChecker {
 protected:
   DirectiveStructureChecker(SemanticsContext &context,
-      std::unordered_map<D, DirectiveClauses<C, ClauseEnumSize>>
-          directiveClausesMap)
+      const std::unordered_map<D, DirectiveClauses<C, ClauseEnumSize>>
+          &directiveClausesMap)
       : context_{context}, directiveClausesMap_(directiveClausesMap) {}
   virtual ~DirectiveStructureChecker() {}
 

@firewave
Copy link

I do not have the full LLVM code at hand but it looks like in some of the cases it should be resolved by moving the parameter instead of making it a const reference.

I filed a downstream ticket with Cppcheck about the problematic suggestion: https://trac.cppcheck.net/ticket/12384.

Also the references to some of the tickets need to be adjusted so they caught when this is merged.

@klausler klausler force-pushed the bug79707 branch 2 times, most recently from 6dbaaf9 to e23a9a5 Compare January 30, 2024 17:54
Four "issues" on GitHub report possible performance problems,
likely detected by static analysis.  None of them would ever
make a measureable difference in compilation time, but I'm
resolving them to clean up the open issues list.

Fixes llvm#79703,
llvm#79705,
llvm#79706, &
llvm#79707.
@klausler klausler merged commit 5a20a20 into llvm:main Feb 20, 2024
4 checks passed
@klausler klausler deleted the bug79707 branch February 20, 2024 22:08
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.

flang/lib/Optimizer/CodeGen/Target.cpp:50: possible performance problem ?
3 participants