From 10db8e816fd25af07240c803449109e26e2a8abf Mon Sep 17 00:00:00 2001 From: Huihui Zhang Date: Mon, 29 Mar 2021 16:37:01 -0700 Subject: [PATCH] [IPO][SampleContextTracker] Use SmallVector to track context profiles to prevent non-determinism. Use SmallVector instead of SmallSet to track the context profiles mapped. Doing this can help avoid non-determinism caused by iterating over unordered containers. This bug was found with reverse iteration turning on, --extra-llvm-cmake-variables="-DLLVM_REVERSE_ITERATION=ON". Failing LLVM test profile-context-tracker-debug.ll . Reviewed By: MaskRay, wenlei Differential Revision: https://reviews.llvm.org/D99547 --- include/llvm/Transforms/IPO/SampleContextTracker.h | 6 +++--- lib/Transforms/IPO/SampleContextTracker.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/llvm/Transforms/IPO/SampleContextTracker.h b/include/llvm/Transforms/IPO/SampleContextTracker.h index 685a060fe46..8566328526a 100644 --- a/include/llvm/Transforms/IPO/SampleContextTracker.h +++ b/include/llvm/Transforms/IPO/SampleContextTracker.h @@ -15,7 +15,7 @@ #ifndef LLVM_TRANSFORMS_IPO_SAMPLECONTEXTTRACKER_H #define LLVM_TRANSFORMS_IPO_SAMPLECONTEXTTRACKER_H -#include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Analysis/CallGraph.h" @@ -91,7 +91,7 @@ private: // calling context and the context is identified by path from root to the node. class SampleContextTracker { public: - using ContextSamplesTy = SmallSet; + using ContextSamplesTy = SmallVector; SampleContextTracker(StringMap &Profiles); // Query context profile for a specific callee with given name at a given @@ -144,7 +144,7 @@ private: StringRef ContextStrToRemove); // Map from function name to context profiles (excluding base profile) - StringMap FuncToCtxtProfileSet; + StringMap FuncToCtxtProfiles; // Root node for context trie tree ContextTrieNode RootContext; diff --git a/lib/Transforms/IPO/SampleContextTracker.cpp b/lib/Transforms/IPO/SampleContextTracker.cpp index 863e8f3833f..36a96e510c7 100644 --- a/lib/Transforms/IPO/SampleContextTracker.cpp +++ b/lib/Transforms/IPO/SampleContextTracker.cpp @@ -183,7 +183,7 @@ SampleContextTracker::SampleContextTracker( SampleContext Context(FuncSample.first(), RawContext); LLVM_DEBUG(dbgs() << "Tracking Context for function: " << Context << "\n"); if (!Context.isBaseContext()) - FuncToCtxtProfileSet[Context.getNameWithoutContext()].insert(FSamples); + FuncToCtxtProfiles[Context.getNameWithoutContext()].push_back(FSamples); ContextTrieNode *NewNode = getOrCreateContextPath(Context, true); assert(!NewNode->getFunctionSamples() && "New node can't have sample profile"); @@ -268,12 +268,12 @@ SampleContextTracker::getContextSamplesFor(const SampleContext &Context) { SampleContextTracker::ContextSamplesTy & SampleContextTracker::getAllContextSamplesFor(const Function &Func) { StringRef CanonName = FunctionSamples::getCanonicalFnName(Func); - return FuncToCtxtProfileSet[CanonName]; + return FuncToCtxtProfiles[CanonName]; } SampleContextTracker::ContextSamplesTy & SampleContextTracker::getAllContextSamplesFor(StringRef Name) { - return FuncToCtxtProfileSet[Name]; + return FuncToCtxtProfiles[Name]; } FunctionSamples *SampleContextTracker::getBaseSamplesFor(const Function &Func, @@ -297,7 +297,7 @@ FunctionSamples *SampleContextTracker::getBaseSamplesFor(StringRef Name, // We have profile for function under different contexts, // create synthetic base profile and merge context profiles // into base profile. - for (auto *CSamples : FuncToCtxtProfileSet[Name]) { + for (auto *CSamples : FuncToCtxtProfiles[Name]) { SampleContext &Context = CSamples->getContext(); ContextTrieNode *FromNode = getContextFor(Context); if (FromNode == Node)