-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
@llvm/pr-subscribers-flang-semantics ChangesTo 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:
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;
};
|
32e4f07
to
43b0c91
Compare
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.
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)
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.