[llvm] [Utils] Consolidate `LockstepReverseIterator` into own header (NFC) (PR #116657)
Antonio Frighetto via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 18 08:59:00 PST 2024
https://github.com/antoniofrighetto created https://github.com/llvm/llvm-project/pull/116657
Common code has been unified and generalized. Not sure if it may be worth to generalize this further, since it looks closely tied Blocks (might make sense to rename it in `LockstepReverseInstructionIterator`).
>From 585631e554f60b15fd83e240ea8dc4a157a057d0 Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me at antoniofrighetto.com>
Date: Mon, 18 Nov 2024 17:54:36 +0100
Subject: [PATCH] [Utils] Consolidate `LockstepReverseIterator` into own header
(NFC)
Common code has been unified and generalized.
---
.../Utils/LockstepReverseIterator.h | 160 ++++++++++++++++++
llvm/lib/Transforms/Scalar/GVNSink.cpp | 94 +---------
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 80 +--------
3 files changed, 171 insertions(+), 163 deletions(-)
create mode 100644 llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
diff --git a/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
new file mode 100644
index 00000000000000..e5fde398f0e4c6
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
@@ -0,0 +1,160 @@
+//===- LockstepReverseIterator.h ------------------------------*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H
+#define LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Instruction.h"
+
+namespace llvm {
+
+struct NoActiveBlocksOption {
+ template <typename... Args> NoActiveBlocksOption(Args...) {}
+};
+
+struct ActiveBlocksOption {
+protected:
+ SmallSetVector<BasicBlock *, 4> ActiveBlocks;
+
+public:
+ ActiveBlocksOption() = default;
+};
+
+/// Iterates through instructions in a set of blocks in reverse order from the
+/// first non-terminator. For example (assume all blocks have size n):
+/// LockstepReverseIterator I([B1, B2, B3]);
+/// *I-- = [B1[n], B2[n], B3[n]];
+/// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
+/// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
+/// ...
+///
+/// The iterator continues processing until all blocks have been exhausted if \p
+/// EarlyFailure is explicitly set to \c false. Use \c getActiveBlocks() to
+/// determine which blocks are still going and the order they appear in the list
+/// returned by operator*.
+template <bool EarlyFailure = true>
+class LockstepReverseIterator
+ : public std::conditional_t<EarlyFailure, NoActiveBlocksOption,
+ ActiveBlocksOption> {
+private:
+ using BasicBlockT = BasicBlock;
+ using InstructionT = Instruction;
+
+ ArrayRef<BasicBlockT *> Blocks;
+ SmallVector<InstructionT *, 4> Insts;
+ bool Fail;
+
+public:
+ LockstepReverseIterator(ArrayRef<BasicBlockT *> Blocks) : Blocks(Blocks) {
+ reset();
+ }
+
+ void reset() {
+ Fail = false;
+ if constexpr (!EarlyFailure) {
+ this->ActiveBlocks.clear();
+ for (BasicBlockT *BB : Blocks)
+ this->ActiveBlocks.insert(BB);
+ }
+ Insts.clear();
+ for (BasicBlockT *BB : Blocks) {
+ InstructionT *Prev = BB->getTerminator()->getPrevNonDebugInstruction();
+ if (!Prev) {
+ // Block wasn't big enough - only contained a terminator.
+ if constexpr (EarlyFailure) {
+ Fail = true;
+ return;
+ } else {
+ this->ActiveBlocks.remove(BB);
+ continue;
+ }
+ }
+ Insts.push_back(Prev);
+ }
+ if (Insts.empty())
+ Fail = true;
+ }
+
+ bool isValid() const { return !Fail; }
+ ArrayRef<InstructionT *> operator*() const { return Insts; }
+
+ // Note: This needs to return a SmallSetVector as the elements of
+ // ActiveBlocks will be later copied to Blocks using std::copy. The
+ // resultant order of elements in Blocks needs to be deterministic.
+ // Using SmallPtrSet instead causes non-deterministic order while
+ // copying. And we cannot simply sort Blocks as they need to match the
+ // corresponding Values.
+ template <bool C = EarlyFailure, std::enable_if_t<!C, int> = 0>
+ SmallSetVector<BasicBlockT *, 4> &getActiveBlocks() {
+ return this->ActiveBlocks;
+ }
+
+ template <bool C = EarlyFailure, std::enable_if_t<!C, int> = 0>
+ void restrictToBlocks(SmallSetVector<BasicBlockT *, 4> &Blocks) {
+ for (auto It = Insts.begin(); It != Insts.end();) {
+ if (!Blocks.contains((*It)->getParent())) {
+ this->ActiveBlocks.remove((*It)->getParent());
+ It = Insts.erase(It);
+ } else {
+ ++It;
+ }
+ }
+ }
+
+ void operator--() {
+ if (Fail)
+ return;
+ SmallVector<InstructionT *, 4> NewInsts;
+ for (InstructionT *Inst : Insts) {
+ InstructionT *Prev = Inst->getPrevNonDebugInstruction();
+ if (!Prev) {
+ if constexpr (!EarlyFailure) {
+ this->ActiveBlocks.remove(Inst->getParent());
+ } else {
+ Fail = true;
+ return;
+ }
+ } else {
+ NewInsts.push_back(Prev);
+ }
+ }
+ if (NewInsts.empty()) {
+ Fail = true;
+ return;
+ }
+ Insts = NewInsts;
+ }
+
+ void operator++() {
+ if (Fail)
+ return;
+ SmallVector<InstructionT *, 4> NewInsts;
+ for (InstructionT *Inst : Insts) {
+ InstructionT *Next = Inst->getNextNonDebugInstruction();
+ // Already at end of block.
+ if (!Next) {
+ Fail = true;
+ return;
+ }
+ NewInsts.push_back(Next);
+ }
+ if (NewInsts.empty()) {
+ Fail = true;
+ return;
+ }
+ Insts = NewInsts;
+ }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_TRANSFORMS_UTILS_LOCKSTEPREVERSEITERATOR_H
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 730f5cd0f8d0d7..8fc09db668fe43 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -65,6 +65,7 @@
#include "llvm/Transforms/Scalar/GVNExpression.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/LockstepReverseIterator.h"
#include <cassert>
#include <cstddef>
#include <cstdint>
@@ -96,87 +97,6 @@ static bool isMemoryInst(const Instruction *I) {
(isa<CallInst>(I) && !cast<CallInst>(I)->doesNotAccessMemory());
}
-/// Iterates through instructions in a set of blocks in reverse order from the
-/// first non-terminator. For example (assume all blocks have size n):
-/// LockstepReverseIterator I([B1, B2, B3]);
-/// *I-- = [B1[n], B2[n], B3[n]];
-/// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
-/// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
-/// ...
-///
-/// It continues until all blocks have been exhausted. Use \c getActiveBlocks()
-/// to
-/// determine which blocks are still going and the order they appear in the
-/// list returned by operator*.
-class LockstepReverseIterator {
- ArrayRef<BasicBlock *> Blocks;
- SmallSetVector<BasicBlock *, 4> ActiveBlocks;
- SmallVector<Instruction *, 4> Insts;
- bool Fail;
-
-public:
- LockstepReverseIterator(ArrayRef<BasicBlock *> Blocks) : Blocks(Blocks) {
- reset();
- }
-
- void reset() {
- Fail = false;
- ActiveBlocks.clear();
- for (BasicBlock *BB : Blocks)
- ActiveBlocks.insert(BB);
- Insts.clear();
- for (BasicBlock *BB : Blocks) {
- if (BB->size() <= 1) {
- // Block wasn't big enough - only contained a terminator.
- ActiveBlocks.remove(BB);
- continue;
- }
- Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction());
- }
- if (Insts.empty())
- Fail = true;
- }
-
- bool isValid() const { return !Fail; }
- ArrayRef<Instruction *> operator*() const { return Insts; }
-
- // Note: This needs to return a SmallSetVector as the elements of
- // ActiveBlocks will be later copied to Blocks using std::copy. The
- // resultant order of elements in Blocks needs to be deterministic.
- // Using SmallPtrSet instead causes non-deterministic order while
- // copying. And we cannot simply sort Blocks as they need to match the
- // corresponding Values.
- SmallSetVector<BasicBlock *, 4> &getActiveBlocks() { return ActiveBlocks; }
-
- void restrictToBlocks(SmallSetVector<BasicBlock *, 4> &Blocks) {
- for (auto II = Insts.begin(); II != Insts.end();) {
- if (!Blocks.contains((*II)->getParent())) {
- ActiveBlocks.remove((*II)->getParent());
- II = Insts.erase(II);
- } else {
- ++II;
- }
- }
- }
-
- void operator--() {
- if (Fail)
- return;
- SmallVector<Instruction *, 4> NewInsts;
- for (auto *Inst : Insts) {
- if (Inst == &Inst->getParent()->front())
- ActiveBlocks.remove(Inst->getParent());
- else
- NewInsts.push_back(Inst->getPrevNonDebugInstruction());
- }
- if (NewInsts.empty()) {
- Fail = true;
- return;
- }
- Insts = NewInsts;
- }
-};
-
//===----------------------------------------------------------------------===//
/// Candidate solution for sinking. There may be different ways to
@@ -634,9 +554,11 @@ class GVNSink {
/// The main heuristic function. Analyze the set of instructions pointed to by
/// LRI and return a candidate solution if these instructions can be sunk, or
/// std::nullopt otherwise.
- std::optional<SinkingInstructionCandidate> analyzeInstructionForSinking(
- LockstepReverseIterator &LRI, unsigned &InstNum, unsigned &MemoryInstNum,
- ModelledPHISet &NeededPHIs, SmallPtrSetImpl<Value *> &PHIContents);
+ std::optional<SinkingInstructionCandidate>
+ analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
+ unsigned &InstNum, unsigned &MemoryInstNum,
+ ModelledPHISet &NeededPHIs,
+ SmallPtrSetImpl<Value *> &PHIContents);
/// Create a ModelledPHI for each PHI in BB, adding to PHIs.
void analyzeInitialPHIs(BasicBlock *BB, ModelledPHISet &PHIs,
@@ -675,7 +597,7 @@ class GVNSink {
};
std::optional<SinkingInstructionCandidate>
-GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
+GVNSink::analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
unsigned &InstNum,
unsigned &MemoryInstNum,
ModelledPHISet &NeededPHIs,
@@ -827,7 +749,7 @@ unsigned GVNSink::sinkBB(BasicBlock *BBEnd) {
return BB->getTerminator()->getNumSuccessors() != 1;
});
- LockstepReverseIterator LRI(Preds);
+ LockstepReverseIterator<false> LRI(Preds);
SmallVector<SinkingInstructionCandidate, 4> Candidates;
unsigned InstNum = 0, MemoryInstNum = 0;
ModelledPHISet NeededPHIs;
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 1991ec82d1e1e4..7440ca46c8d27d 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -74,6 +74,7 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/LockstepReverseIterator.h"
#include "llvm/Transforms/Utils/ValueMapper.h"
#include <algorithm>
#include <cassert>
@@ -2326,81 +2327,6 @@ static void sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
}
}
-namespace {
-
- // LockstepReverseIterator - Iterates through instructions
- // in a set of blocks in reverse order from the first non-terminator.
- // For example (assume all blocks have size n):
- // LockstepReverseIterator I([B1, B2, B3]);
- // *I-- = [B1[n], B2[n], B3[n]];
- // *I-- = [B1[n-1], B2[n-1], B3[n-1]];
- // *I-- = [B1[n-2], B2[n-2], B3[n-2]];
- // ...
- class LockstepReverseIterator {
- ArrayRef<BasicBlock*> Blocks;
- SmallVector<Instruction*,4> Insts;
- bool Fail;
-
- public:
- LockstepReverseIterator(ArrayRef<BasicBlock*> Blocks) : Blocks(Blocks) {
- reset();
- }
-
- void reset() {
- Fail = false;
- Insts.clear();
- for (auto *BB : Blocks) {
- Instruction *Inst = BB->getTerminator();
- for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
- Inst = Inst->getPrevNode();
- if (!Inst) {
- // Block wasn't big enough.
- Fail = true;
- return;
- }
- Insts.push_back(Inst);
- }
- }
-
- bool isValid() const {
- return !Fail;
- }
-
- void operator--() {
- if (Fail)
- return;
- for (auto *&Inst : Insts) {
- for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
- Inst = Inst->getPrevNode();
- // Already at beginning of block.
- if (!Inst) {
- Fail = true;
- return;
- }
- }
- }
-
- void operator++() {
- if (Fail)
- return;
- for (auto *&Inst : Insts) {
- for (Inst = Inst->getNextNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
- Inst = Inst->getNextNode();
- // Already at end of block.
- if (!Inst) {
- Fail = true;
- return;
- }
- }
- }
-
- ArrayRef<Instruction*> operator * () const {
- return Insts;
- }
- };
-
-} // end anonymous namespace
-
/// Check whether BB's predecessors end with unconditional branches. If it is
/// true, sink any common code from the predecessors to BB.
static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
@@ -2477,7 +2403,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
int ScanIdx = 0;
SmallPtrSet<Value*,4> InstructionsToSink;
- LockstepReverseIterator LRI(UnconditionalPreds);
+ LockstepReverseIterator<true> LRI(UnconditionalPreds);
while (LRI.isValid() &&
canSinkInstructions(*LRI, PHIOperands)) {
LLVM_DEBUG(dbgs() << "SINK: instruction can be sunk: " << *(*LRI)[0]
@@ -2507,7 +2433,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
// Okay, we *could* sink last ScanIdx instructions. But how many can we
// actually sink before encountering instruction that is unprofitable to
// sink?
- auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) {
+ auto ProfitableToSinkInstruction = [&](LockstepReverseIterator<true> &LRI) {
unsigned NumPHIInsts = 0;
for (Use &U : (*LRI)[0]->operands()) {
auto It = PHIOperands.find(&U);
More information about the llvm-commits
mailing list