From 5e908e3bab693adbf84f78c078eb17b63e4c2a7a Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Mon, 20 Jan 2014 14:03:02 +0000 Subject: [PATCH] Track multiple stores per object when using AA in ScheduleDAGInstrs When using AA to break false chain dependencies, we need to track multiple stores per object in ScheduleDAGInstrs. Historically, we tracked potential alias chains at the object level, and so all loads of an object would retain dependencies on any store to that object. With AA, however, this is not sufficient: non-overlapping stores and loads to the same object all need to be tested for dependencies separately, we cannot only test all loads to an object against only the last store (see PR18497 for an explicit example). To mitigate any unwelcome compile-time impact when not using AA, only one store is kept in the list per object when not using AA. This, along with a stack coloring change to come shortly, will provide a test case, fix PR18497 (and allow LLVM to compile itself using -enable-aa-sched-mi on x86-64). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@199657 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/ScheduleDAGInstrs.cpp | 59 ++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/lib/CodeGen/ScheduleDAGInstrs.cpp b/lib/CodeGen/ScheduleDAGInstrs.cpp index 65075893e5a..93ee38bc0bf 100644 --- a/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -747,7 +747,7 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, // so that they can be given more precise dependencies. We track // separately the known memory locations that may alias and those // that are known not to alias - MapVector AliasMemDefs, NonAliasMemDefs; + MapVector > AliasMemDefs, NonAliasMemDefs; MapVector > AliasMemUses, NonAliasMemUses; std::set RejectMemNodes; @@ -842,9 +842,11 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, if (isGlobalMemoryObject(AA, MI)) { // Be conservative with these and add dependencies on all memory // references, even those that are known to not alias. - for (MapVector::iterator I = + for (MapVector >::iterator I = NonAliasMemDefs.begin(), E = NonAliasMemDefs.end(); I != E; ++I) { - I->second->addPred(SDep(SU, SDep::Barrier)); + for (unsigned i = 0, e = I->second.size(); i != e; ++i) { + I->second[i]->addPred(SDep(SU, SDep::Barrier)); + } } for (MapVector >::iterator I = NonAliasMemUses.begin(), E = NonAliasMemUses.end(); I != E; ++I) { @@ -880,9 +882,11 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, for (unsigned k = 0, m = PendingLoads.size(); k != m; ++k) addChainDependency(AAForDep, MFI, SU, PendingLoads[k], RejectMemNodes, TrueMemOrderLatency); - for (MapVector::iterator I = AliasMemDefs.begin(), - E = AliasMemDefs.end(); I != E; ++I) - addChainDependency(AAForDep, MFI, SU, I->second, RejectMemNodes); + for (MapVector >::iterator I = + AliasMemDefs.begin(), E = AliasMemDefs.end(); I != E; ++I) { + for (unsigned i = 0, e = I->second.size(); i != e; ++i) + addChainDependency(AAForDep, MFI, SU, I->second[i], RejectMemNodes); + } for (MapVector >::iterator I = AliasMemUses.begin(), E = AliasMemUses.end(); I != E; ++I) { for (unsigned i = 0, e = I->second.size(); i != e; ++i) @@ -914,19 +918,29 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, // A store to a specific PseudoSourceValue. Add precise dependencies. // Record the def in MemDefs, first adding a dep if there is // an existing def. - MapVector::iterator I = + MapVector >::iterator I = ((ThisMayAlias) ? AliasMemDefs.find(V) : NonAliasMemDefs.find(V)); - MapVector::iterator IE = + MapVector >::iterator IE = ((ThisMayAlias) ? AliasMemDefs.end() : NonAliasMemDefs.end()); if (I != IE) { - addChainDependency(AAForDep, MFI, SU, I->second, RejectMemNodes, - 0, true); - I->second = SU; + for (unsigned i = 0, e = I->second.size(); i != e; ++i) + addChainDependency(AAForDep, MFI, SU, I->second[i], RejectMemNodes, + 0, true); + + // If we're not using AA, then we only need one store per object. + if (!AAForDep) + I->second.clear(); + I->second.push_back(SU); } else { - if (ThisMayAlias) - AliasMemDefs[V] = SU; - else - NonAliasMemDefs[V] = SU; + if (ThisMayAlias) { + if (!AAForDep) + AliasMemDefs[V].clear(); + AliasMemDefs[V].push_back(SU); + } else { + if (!AAForDep) + NonAliasMemDefs[V].clear(); + NonAliasMemDefs[V].push_back(SU); + } } // Handle the uses in MemUses, if there are any. MapVector >::iterator J = @@ -976,9 +990,11 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, if (Objs.empty()) { // A load with no underlying object. Depend on all // potentially aliasing stores. - for (MapVector::iterator I = + for (MapVector >::iterator I = AliasMemDefs.begin(), E = AliasMemDefs.end(); I != E; ++I) - addChainDependency(AAForDep, MFI, SU, I->second, RejectMemNodes); + for (unsigned i = 0, e = I->second.size(); i != e; ++i) + addChainDependency(AAForDep, MFI, SU, I->second[i], + RejectMemNodes); PendingLoads.push_back(SU); MayAlias = true; @@ -995,13 +1011,14 @@ void ScheduleDAGInstrs::buildSchedGraph(AliasAnalysis *AA, MayAlias = true; // A load from a specific PseudoSourceValue. Add precise dependencies. - MapVector::iterator I = + MapVector >::iterator I = ((ThisMayAlias) ? AliasMemDefs.find(V) : NonAliasMemDefs.find(V)); - MapVector::iterator IE = + MapVector >::iterator IE = ((ThisMayAlias) ? AliasMemDefs.end() : NonAliasMemDefs.end()); if (I != IE) - addChainDependency(AAForDep, MFI, SU, I->second, RejectMemNodes, - 0, true); + for (unsigned i = 0, e = I->second.size(); i != e; ++i) + addChainDependency(AAForDep, MFI, SU, I->second[i], + RejectMemNodes, 0, true); if (ThisMayAlias) AliasMemUses[V].push_back(SU); else -- 2.34.1