Skip to content

[flang][NFC] Speed up large DATA statement initializations #67585

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
merged 1 commit into from
Oct 16, 2023

Conversation

klausler
Copy link
Contributor

To ensure that the map from symbols to their initial images has an entry for a particular symbol, use std::map<>::find() before std::map<>::emplace() to avoid needless memory allocation and deallocation. Also, combine adjacent intervals in the lists of initialized ranges so that contiguous initializations don't require long lists.

Fixes #66452.

@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2023

@llvm/pr-subscribers-flang-semantics

Changes

To ensure that the map from symbols to their initial images has an entry for a particular symbol, use std::map<>::find() before std::map<>::emplace() to avoid needless memory allocation and deallocation. Also, combine adjacent intervals in the lists of initialized ranges so that contiguous initializations don't require long lists.

Fixes #66452.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/data-to-inits.cpp (+16-13)
  • (modified) flang/lib/Semantics/data-to-inits.h (+16)
diff --git a/flang/lib/Semantics/data-to-inits.cpp b/flang/lib/Semantics/data-to-inits.cpp
index bc0355a2c597a6f..85bce874e78cdeb 100644
--- a/flang/lib/Semantics/data-to-inits.cpp
+++ b/flang/lib/Semantics/data-to-inits.cpp
@@ -81,7 +81,7 @@ template <typename DSV = parser::DataStmtValue> class ValueListIterator {
 };
 
 template <typename DSV> void ValueListIterator<DSV>::SetRepetitionCount() {
-  for (repetitionsRemaining_ = 1; at_ != end_; ++at_) {
+  for (; at_ != end_; ++at_) {
     auto repetitions{GetValue().repetitions};
     if (repetitions < 0) {
       hasFatalError_ = true;
@@ -335,10 +335,15 @@ bool DataInitializationCompiler<DSV>::InitElement(
     }
   }};
   const auto GetImage{[&]() -> evaluate::InitialImage & {
-    auto iter{inits_.emplace(&symbol, symbol.size())};
-    auto &symbolInit{iter.first->second};
-    symbolInit.initializedRanges.emplace_back(
-        offsetSymbol.offset(), offsetSymbol.size());
+    // This could be (and was) written to always call std::map<>::emplace(),
+    // which should handle duplicate entries gracefully, but it was still
+    // causing memory allocation & deallocation with gcc.
+    auto iter{inits_.find(&symbol)};
+    if (iter == inits_.end()) {
+      iter = inits_.emplace(&symbol, symbol.size()).first;
+    }
+    auto &symbolInit{iter->second};
+    symbolInit.NoteInitializedRange(offsetSymbol);
     return symbolInit.image;
   }};
   const auto OutOfRangeError{[&]() {
@@ -590,8 +595,7 @@ static void PopulateWithComponentDefaults(SymbolDataInitialization &init,
           }
         }
         if (initialized) {
-          init.initializedRanges.emplace_back(
-              componentOffset, component.size());
+          init.NoteInitializedRange(componentOffset, component.size());
         }
       }
     } else if (const auto *proc{component.detailsIf<ProcEntityDetails>()}) {
@@ -599,8 +603,7 @@ static void PopulateWithComponentDefaults(SymbolDataInitialization &init,
         SomeExpr procPtrInit{evaluate::ProcedureDesignator{**proc->init()}};
         auto extant{init.image.AsConstantPointer(componentOffset)};
         if (!extant || !(*extant == procPtrInit)) {
-          init.initializedRanges.emplace_back(
-              componentOffset, component.size());
+          init.NoteInitializedRange(componentOffset, component.size());
           init.image.AddPointer(componentOffset, std::move(procPtrInit));
         }
       }
@@ -651,7 +654,7 @@ static void IncorporateExplicitInitialization(
   if (iter != inits.end()) { // DATA statement initialization
     for (const auto &range : iter->second.initializedRanges) {
       auto at{offset + range.start()};
-      combined.initializedRanges.emplace_back(at, range.size());
+      combined.NoteInitializedRange(at, range.size());
       combined.image.Incorporate(
           at, iter->second.image, range.start(), range.size());
     }
@@ -663,7 +666,7 @@ static void IncorporateExplicitInitialization(
     if (IsPointer(mutableSymbol)) {
       if (auto *object{mutableSymbol.detailsIf<ObjectEntityDetails>()}) {
         if (object->init()) {
-          combined.initializedRanges.emplace_back(offset, mutableSymbol.size());
+          combined.NoteInitializedRange(offset, mutableSymbol.size());
           combined.image.AddPointer(offset, *object->init());
           if (removeOriginalInits) {
             object->init().reset();
@@ -671,7 +674,7 @@ static void IncorporateExplicitInitialization(
         }
       } else if (auto *proc{mutableSymbol.detailsIf<ProcEntityDetails>()}) {
         if (proc->init() && *proc->init()) {
-          combined.initializedRanges.emplace_back(offset, mutableSymbol.size());
+          combined.NoteInitializedRange(offset, mutableSymbol.size());
           combined.image.AddPointer(
               offset, SomeExpr{evaluate::ProcedureDesignator{**proc->init()}});
           if (removeOriginalInits) {
@@ -681,7 +684,7 @@ static void IncorporateExplicitInitialization(
       }
     } else if (auto *object{mutableSymbol.detailsIf<ObjectEntityDetails>()}) {
       if (!IsNamedConstant(mutableSymbol) && object->init()) {
-        combined.initializedRanges.emplace_back(offset, mutableSymbol.size());
+        combined.NoteInitializedRange(offset, mutableSymbol.size());
         combined.image.Add(
             offset, mutableSymbol.size(), *object->init(), foldingContext);
         if (removeOriginalInits) {
diff --git a/flang/lib/Semantics/data-to-inits.h b/flang/lib/Semantics/data-to-inits.h
index 10d850d23d5d636..d8cc4601de26fa9 100644
--- a/flang/lib/Semantics/data-to-inits.h
+++ b/flang/lib/Semantics/data-to-inits.h
@@ -11,6 +11,7 @@
 
 #include "flang/Common/default-kinds.h"
 #include "flang/Common/interval.h"
+#include "flang/Evaluate/fold-designator.h"
 #include "flang/Evaluate/initial-image.h"
 #include <list>
 #include <map>
@@ -30,6 +31,21 @@ struct SymbolDataInitialization {
   using Range = common::Interval<common::ConstantSubscript>;
   explicit SymbolDataInitialization(std::size_t bytes) : image{bytes} {}
   SymbolDataInitialization(SymbolDataInitialization &&) = default;
+
+  void NoteInitializedRange(Range range) {
+    if (initializedRanges.empty() ||
+        !initializedRanges.back().AnnexIfPredecessor(range)) {
+      initializedRanges.emplace_back(range);
+    }
+  }
+  void NoteInitializedRange(
+      common::ConstantSubscript offset, std::size_t size) {
+    NoteInitializedRange(Range{offset, size});
+  }
+  void NoteInitializedRange(evaluate::OffsetSymbol offsetSymbol) {
+    NoteInitializedRange(offsetSymbol.offset(), offsetSymbol.size());
+  }
+
   evaluate::InitialImage image;
   std::list<Range> initializedRanges;
 };

@klausler klausler force-pushed the bug66452 branch 2 times, most recently from 32e4f07 to 43b0c91 Compare September 29, 2023 21:19
To ensure that the map from symbols to their initial images
has an entry for a particular symbol, use std::map<>::find()
before std::map<>::emplace() to avoid needless memory allocation
and deallocation.  Also, combine adjacent intervals in the lists
of initialized ranges so that contiguous initializations don't
require long lists.

Fixes llvm#66452.
@klausler klausler merged commit 28a686a into llvm:main Oct 16, 2023
@klausler klausler deleted the bug66452 branch October 16, 2023 23:51
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 20, 2023
Local branch amd-gfx ae5318a Merged main:233c3e6c53a5 into amd-gfx:8892e09cb17c
Remote branch main 28a686a [flang][NFC] Speed up large DATA statement initializations (llvm#67585)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flang] Compilation of DATA statement for large COMPLEX arrays needs a lot of time
2 participants