Skip to content

Commit 236fa50

Browse files
committed
Revert "[Utils] Consolidate LockstepReverseIterator into own header (NFC) (#116657)"
This reverts commit 123dca9. This breaks building on macOS with clang and multiple build bots, including https://lab.llvm.org/buildbot/#/builders/175/builds/13585 llvm-project/llvm/lib/Transforms/Utils/SimplifyCFG.cpp: In function ‘bool sinkCommonCodeFromPredecessors(llvm::BasicBlock*, llvm::DomTreeUpdater*)’: /b/ml-opt-devrel-x86-64-b1/llvm-project/llvm/lib/Transforms/Utils/SimplifyCFG.cpp:2503:3: error: reference to ‘LockstepReverseIterator’ is ambiguous 2503 | LockstepReverseIterator<true> LRI(UnconditionalPreds); | ^~~~~~~~~~~~~~~~~~~~~~~
1 parent c3d5070 commit 236fa50

File tree

3 files changed

+88
-166
lines changed

3 files changed

+88
-166
lines changed

llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h

Lines changed: 0 additions & 155 deletions
This file was deleted.

llvm/lib/Transforms/Scalar/GVNSink.cpp

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@
6565
#include "llvm/Transforms/Scalar/GVNExpression.h"
6666
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
6767
#include "llvm/Transforms/Utils/Local.h"
68-
#include "llvm/Transforms/Utils/LockstepReverseIterator.h"
6968
#include <cassert>
7069
#include <cstddef>
7170
#include <cstdint>
@@ -97,6 +96,87 @@ static bool isMemoryInst(const Instruction *I) {
9796
(isa<CallInst>(I) && !cast<CallInst>(I)->doesNotAccessMemory());
9897
}
9998

99+
/// Iterates through instructions in a set of blocks in reverse order from the
100+
/// first non-terminator. For example (assume all blocks have size n):
101+
/// LockstepReverseIterator I([B1, B2, B3]);
102+
/// *I-- = [B1[n], B2[n], B3[n]];
103+
/// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
104+
/// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
105+
/// ...
106+
///
107+
/// It continues until all blocks have been exhausted. Use \c getActiveBlocks()
108+
/// to
109+
/// determine which blocks are still going and the order they appear in the
110+
/// list returned by operator*.
111+
class LockstepReverseIterator {
112+
ArrayRef<BasicBlock *> Blocks;
113+
SmallSetVector<BasicBlock *, 4> ActiveBlocks;
114+
SmallVector<Instruction *, 4> Insts;
115+
bool Fail;
116+
117+
public:
118+
LockstepReverseIterator(ArrayRef<BasicBlock *> Blocks) : Blocks(Blocks) {
119+
reset();
120+
}
121+
122+
void reset() {
123+
Fail = false;
124+
ActiveBlocks.clear();
125+
for (BasicBlock *BB : Blocks)
126+
ActiveBlocks.insert(BB);
127+
Insts.clear();
128+
for (BasicBlock *BB : Blocks) {
129+
if (BB->size() <= 1) {
130+
// Block wasn't big enough - only contained a terminator.
131+
ActiveBlocks.remove(BB);
132+
continue;
133+
}
134+
Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction());
135+
}
136+
if (Insts.empty())
137+
Fail = true;
138+
}
139+
140+
bool isValid() const { return !Fail; }
141+
ArrayRef<Instruction *> operator*() const { return Insts; }
142+
143+
// Note: This needs to return a SmallSetVector as the elements of
144+
// ActiveBlocks will be later copied to Blocks using std::copy. The
145+
// resultant order of elements in Blocks needs to be deterministic.
146+
// Using SmallPtrSet instead causes non-deterministic order while
147+
// copying. And we cannot simply sort Blocks as they need to match the
148+
// corresponding Values.
149+
SmallSetVector<BasicBlock *, 4> &getActiveBlocks() { return ActiveBlocks; }
150+
151+
void restrictToBlocks(SmallSetVector<BasicBlock *, 4> &Blocks) {
152+
for (auto II = Insts.begin(); II != Insts.end();) {
153+
if (!Blocks.contains((*II)->getParent())) {
154+
ActiveBlocks.remove((*II)->getParent());
155+
II = Insts.erase(II);
156+
} else {
157+
++II;
158+
}
159+
}
160+
}
161+
162+
void operator--() {
163+
if (Fail)
164+
return;
165+
SmallVector<Instruction *, 4> NewInsts;
166+
for (auto *Inst : Insts) {
167+
if (Inst == &Inst->getParent()->front())
168+
ActiveBlocks.remove(Inst->getParent());
169+
else
170+
NewInsts.push_back(Inst->getPrevNonDebugInstruction());
171+
}
172+
if (NewInsts.empty()) {
173+
Fail = true;
174+
return;
175+
}
176+
Insts = NewInsts;
177+
}
178+
};
179+
100180
//===----------------------------------------------------------------------===//
101181

102182
/// Candidate solution for sinking. There may be different ways to
@@ -554,11 +634,9 @@ class GVNSink {
554634
/// The main heuristic function. Analyze the set of instructions pointed to by
555635
/// LRI and return a candidate solution if these instructions can be sunk, or
556636
/// std::nullopt otherwise.
557-
std::optional<SinkingInstructionCandidate>
558-
analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
559-
unsigned &InstNum, unsigned &MemoryInstNum,
560-
ModelledPHISet &NeededPHIs,
561-
SmallPtrSetImpl<Value *> &PHIContents);
637+
std::optional<SinkingInstructionCandidate> analyzeInstructionForSinking(
638+
LockstepReverseIterator &LRI, unsigned &InstNum, unsigned &MemoryInstNum,
639+
ModelledPHISet &NeededPHIs, SmallPtrSetImpl<Value *> &PHIContents);
562640

563641
/// Create a ModelledPHI for each PHI in BB, adding to PHIs.
564642
void analyzeInitialPHIs(BasicBlock *BB, ModelledPHISet &PHIs,
@@ -597,7 +675,7 @@ class GVNSink {
597675
};
598676

599677
std::optional<SinkingInstructionCandidate>
600-
GVNSink::analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
678+
GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
601679
unsigned &InstNum,
602680
unsigned &MemoryInstNum,
603681
ModelledPHISet &NeededPHIs,
@@ -749,7 +827,7 @@ unsigned GVNSink::sinkBB(BasicBlock *BBEnd) {
749827
return BB->getTerminator()->getNumSuccessors() != 1;
750828
});
751829

752-
LockstepReverseIterator<false> LRI(Preds);
830+
LockstepReverseIterator LRI(Preds);
753831
SmallVector<SinkingInstructionCandidate, 4> Candidates;
754832
unsigned InstNum = 0, MemoryInstNum = 0;
755833
ModelledPHISet NeededPHIs;

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@
7474
#include "llvm/Support/raw_ostream.h"
7575
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
7676
#include "llvm/Transforms/Utils/Local.h"
77-
#include "llvm/Transforms/Utils/LockstepReverseIterator.h"
7877
#include "llvm/Transforms/Utils/ValueMapper.h"
7978
#include <algorithm>
8079
#include <cassert>
@@ -2500,7 +2499,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
25002499

25012500
int ScanIdx = 0;
25022501
SmallPtrSet<Value*,4> InstructionsToSink;
2503-
LockstepReverseIterator<true> LRI(UnconditionalPreds);
2502+
LockstepReverseIterator LRI(UnconditionalPreds);
25042503
while (LRI.isValid() &&
25052504
canSinkInstructions(*LRI, PHIOperands)) {
25062505
LLVM_DEBUG(dbgs() << "SINK: instruction can be sunk: " << *(*LRI)[0]
@@ -2530,7 +2529,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
25302529
// Okay, we *could* sink last ScanIdx instructions. But how many can we
25312530
// actually sink before encountering instruction that is unprofitable to
25322531
// sink?
2533-
auto ProfitableToSinkInstruction = [&](LockstepReverseIterator<true> &LRI) {
2532+
auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) {
25342533
unsigned NumPHIInsts = 0;
25352534
for (Use &U : (*LRI)[0]->operands()) {
25362535
auto It = PHIOperands.find(&U);

0 commit comments

Comments
 (0)