Skip to content

Commit 48a6df3

Browse files
antoniofrighettoantoniofrighetto
committed
Reapply "[Utils] Consolidate LockstepReverseIterator into own header (NFC)"
Common code has been unified and generalized. Original commit: 123dca9 Previously reverted due to accidentally merged incompletely. The issue has been addressed by restoring missing code.
1 parent b09dfbd commit 48a6df3

File tree

3 files changed

+166
-88
lines changed

3 files changed

+166
-88
lines changed

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

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
//===- LockstepReverseIterator.h ------------------------------*- C++ -*---===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H
10+
#define LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H
11+
12+
#include "llvm/ADT/ArrayRef.h"
13+
#include "llvm/ADT/SetVector.h"
14+
#include "llvm/ADT/SmallVector.h"
15+
#include "llvm/IR/BasicBlock.h"
16+
#include "llvm/IR/Instruction.h"
17+
18+
namespace llvm {
19+
20+
struct NoActiveBlocksOption {};
21+
22+
struct ActiveBlocksOption {
23+
SmallSetVector<BasicBlock *, 4> ActiveBlocks;
24+
SmallSetVector<BasicBlock *, 4> &getActiveBlocks() { return ActiveBlocks; }
25+
ActiveBlocksOption() = default;
26+
};
27+
28+
/// Iterates through instructions in a set of blocks in reverse order from the
29+
/// first non-terminator. For example (assume all blocks have size n):
30+
/// LockstepReverseIterator I([B1, B2, B3]);
31+
/// *I-- = [B1[n], B2[n], B3[n]];
32+
/// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
33+
/// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
34+
/// ...
35+
///
36+
/// The iterator continues processing until all blocks have been exhausted if \p
37+
/// EarlyFailure is explicitly set to \c false. Use \c getActiveBlocks() to
38+
/// determine which blocks are still going and the order they appear in the list
39+
/// returned by operator*.
40+
template <bool EarlyFailure = true>
41+
class LockstepReverseIterator
42+
: private std::conditional_t<EarlyFailure, NoActiveBlocksOption,
43+
ActiveBlocksOption> {
44+
private:
45+
using Base = std::conditional_t<EarlyFailure, NoActiveBlocksOption,
46+
ActiveBlocksOption>;
47+
ArrayRef<BasicBlock *> Blocks;
48+
SmallVector<Instruction *, 4> Insts;
49+
bool Fail;
50+
51+
public:
52+
LockstepReverseIterator(ArrayRef<BasicBlock *> Blocks) : Blocks(Blocks) {
53+
reset();
54+
}
55+
56+
void reset() {
57+
Fail = false;
58+
if constexpr (!EarlyFailure) {
59+
this->ActiveBlocks.clear();
60+
for (BasicBlock *BB : Blocks)
61+
this->ActiveBlocks.insert(BB);
62+
}
63+
Insts.clear();
64+
for (BasicBlock *BB : Blocks) {
65+
Instruction *Prev = BB->getTerminator()->getPrevNonDebugInstruction();
66+
if (!Prev) {
67+
// Block wasn't big enough - only contained a terminator.
68+
if constexpr (EarlyFailure) {
69+
Fail = true;
70+
return;
71+
} else {
72+
this->ActiveBlocks.remove(BB);
73+
continue;
74+
}
75+
}
76+
Insts.push_back(Prev);
77+
}
78+
if (Insts.empty())
79+
Fail = true;
80+
}
81+
82+
bool isValid() const { return !Fail; }
83+
ArrayRef<Instruction *> operator*() const { return Insts; }
84+
85+
// Note: This needs to return a SmallSetVector as the elements of
86+
// ActiveBlocks will be later copied to Blocks using std::copy. The
87+
// resultant order of elements in Blocks needs to be deterministic.
88+
// Using SmallPtrSet instead causes non-deterministic order while
89+
// copying. And we cannot simply sort Blocks as they need to match the
90+
// corresponding Values.
91+
SmallSetVector<BasicBlock *, 4> &getActiveBlocks() {
92+
return Base::getActiveBlocks();
93+
}
94+
95+
void restrictToBlocks(SmallSetVector<BasicBlock *, 4> &Blocks) {
96+
static_assert(!EarlyFailure, "Unknown method");
97+
for (auto It = Insts.begin(); It != Insts.end();) {
98+
if (!Blocks.contains((*It)->getParent())) {
99+
this->ActiveBlocks.remove((*It)->getParent());
100+
It = Insts.erase(It);
101+
} else {
102+
++It;
103+
}
104+
}
105+
}
106+
107+
LockstepReverseIterator &operator--() {
108+
if (Fail)
109+
return *this;
110+
SmallVector<Instruction *, 4> NewInsts;
111+
for (Instruction *Inst : Insts) {
112+
Instruction *Prev = Inst->getPrevNonDebugInstruction();
113+
if (!Prev) {
114+
if constexpr (!EarlyFailure) {
115+
this->ActiveBlocks.remove(Inst->getParent());
116+
} else {
117+
Fail = true;
118+
return *this;
119+
}
120+
} else {
121+
NewInsts.push_back(Prev);
122+
}
123+
}
124+
if (NewInsts.empty())
125+
Fail = true;
126+
else
127+
Insts = NewInsts;
128+
return *this;
129+
}
130+
131+
LockstepReverseIterator &operator++() {
132+
static_assert(EarlyFailure, "Unknown method");
133+
if (Fail)
134+
return *this;
135+
SmallVector<Instruction *, 4> NewInsts;
136+
for (Instruction *Inst : Insts) {
137+
Instruction *Next = Inst->getNextNonDebugInstruction();
138+
// Already at end of block.
139+
if (!Next) {
140+
Fail = true;
141+
return *this;
142+
}
143+
NewInsts.push_back(Next);
144+
}
145+
if (NewInsts.empty())
146+
Fail = true;
147+
else
148+
Insts = NewInsts;
149+
return *this;
150+
}
151+
};
152+
153+
} // end namespace llvm
154+
155+
#endif // LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H

‎llvm/lib/Transforms/Scalar/GVNSink.cpp

Lines changed: 8 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
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"
6869
#include <cassert>
6970
#include <cstddef>
7071
#include <cstdint>
@@ -96,87 +97,6 @@ static bool isMemoryInst(const Instruction *I) {
9697
(isa<CallInst>(I) && !cast<CallInst>(I)->doesNotAccessMemory());
9798
}
9899

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-
180100
//===----------------------------------------------------------------------===//
181101

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

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

677599
std::optional<SinkingInstructionCandidate>
678-
GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
600+
GVNSink::analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
679601
unsigned &InstNum,
680602
unsigned &MemoryInstNum,
681603
ModelledPHISet &NeededPHIs,
@@ -827,7 +749,7 @@ unsigned GVNSink::sinkBB(BasicBlock *BBEnd) {
827749
return BB->getTerminator()->getNumSuccessors() != 1;
828750
});
829751

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

‎llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
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"
7778
#include "llvm/Transforms/Utils/ValueMapper.h"
7879
#include <algorithm>
7980
#include <cassert>
@@ -2499,7 +2500,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
24992500

25002501
int ScanIdx = 0;
25012502
SmallPtrSet<Value*,4> InstructionsToSink;
2502-
LockstepReverseIterator LRI(UnconditionalPreds);
2503+
LockstepReverseIterator<true> LRI(UnconditionalPreds);
25032504
while (LRI.isValid() &&
25042505
canSinkInstructions(*LRI, PHIOperands)) {
25052506
LLVM_DEBUG(dbgs() << "SINK: instruction can be sunk: " << *(*LRI)[0]
@@ -2529,7 +2530,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
25292530
// Okay, we *could* sink last ScanIdx instructions. But how many can we
25302531
// actually sink before encountering instruction that is unprofitable to
25312532
// sink?
2532-
auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) {
2533+
auto ProfitableToSinkInstruction = [&](LockstepReverseIterator<true> &LRI) {
25332534
unsigned NumPHIInsts = 0;
25342535
for (Use &U : (*LRI)[0]->operands()) {
25352536
auto It = PHIOperands.find(&U);

0 commit comments

Comments
 (0)