[flang-commits] [flang] [flang][NFC] Speed up large DATA statement initializations (PR #67585)

[Peter Klausler via flang-commits]

https://github.com/klausler updated https://github.com/llvm/llvm-project/pull/67585

>From ef90593387a8da7d3b07669f18f23ae03908431d Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Wed, 27 Sep 2023 11:09:54 -0700
Subject: [PATCH] [flang][NFC] Speed up large DATA statement initializations

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 https://github.com/llvm/llvm-project/issues/66452.
---
 flang/lib/Semantics/data-to-inits.cpp | 29 +++++++++++++++------------
 flang/lib/Semantics/data-to-inits.h   | 16 +++++++++++++++
 2 files changed, 32 insertions(+), 13 deletions(-)

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;
 };