[llvm] 123dca9 - [Utils] Consolidate `LockstepReverseIterator` into own header (NFC) (#116657)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 21 12:21:36 PST 2025
Author: Antonio Frighetto
Date: 2025-02-21T12:21:33-08:00
New Revision: 123dca9b56e1359d8ec7771ea3bd0afd4b1ea6af
URL: https://github.com/llvm/llvm-project/commit/123dca9b56e1359d8ec7771ea3bd0afd4b1ea6af
DIFF: https://github.com/llvm/llvm-project/commit/123dca9b56e1359d8ec7771ea3bd0afd4b1ea6af.diff
LOG: [Utils] Consolidate `LockstepReverseIterator` into own header (NFC) (#116657)
Common code has been unified and generalized. Not sure if it may be
worth to generalize this further, since it looks closely tied to Blocks
(might make sense to rename it in `LockstepReverseInstructionIterator`).
Added:
llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
Modified:
llvm/lib/Transforms/Scalar/GVNSink.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
new file mode 100644
index 0000000000000..cc9d717054528
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
@@ -0,0 +1,155 @@
+//===- 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 {};
+
+struct ActiveBlocksOption {
+ SmallSetVector<BasicBlock *, 4> ActiveBlocks;
+ SmallSetVector<BasicBlock *, 4> &getActiveBlocks() { return ActiveBlocks; }
+ 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
+ : private std::conditional_t<EarlyFailure, NoActiveBlocksOption,
+ ActiveBlocksOption> {
+private:
+ using Base = std::conditional_t<EarlyFailure, NoActiveBlocksOption,
+ ActiveBlocksOption>;
+ ArrayRef<BasicBlock *> Blocks;
+ SmallVector<Instruction *, 4> Insts;
+ bool Fail;
+
+public:
+ LockstepReverseIterator(ArrayRef<BasicBlock *> Blocks) : Blocks(Blocks) {
+ reset();
+ }
+
+ void reset() {
+ Fail = false;
+ if constexpr (!EarlyFailure) {
+ this->ActiveBlocks.clear();
+ for (BasicBlock *BB : Blocks)
+ this->ActiveBlocks.insert(BB);
+ }
+ Insts.clear();
+ for (BasicBlock *BB : Blocks) {
+ Instruction *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<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 Base::getActiveBlocks();
+ }
+
+ void restrictToBlocks(SmallSetVector<BasicBlock *, 4> &Blocks) {
+ static_assert(!EarlyFailure, "Unknown method");
+ for (auto It = Insts.begin(); It != Insts.end();) {
+ if (!Blocks.contains((*It)->getParent())) {
+ this->ActiveBlocks.remove((*It)->getParent());
+ It = Insts.erase(It);
+ } else {
+ ++It;
+ }
+ }
+ }
+
+ LockstepReverseIterator &operator--() {
+ if (Fail)
+ return *this;
+ SmallVector<Instruction *, 4> NewInsts;
+ for (Instruction *Inst : Insts) {
+ Instruction *Prev = Inst->getPrevNonDebugInstruction();
+ if (!Prev) {
+ if constexpr (!EarlyFailure) {
+ this->ActiveBlocks.remove(Inst->getParent());
+ } else {
+ Fail = true;
+ return *this;
+ }
+ } else {
+ NewInsts.push_back(Prev);
+ }
+ }
+ if (NewInsts.empty())
+ Fail = true;
+ else
+ Insts = NewInsts;
+ return *this;
+ }
+
+ LockstepReverseIterator &operator++() {
+ static_assert(EarlyFailure, "Unknown method");
+ if (Fail)
+ return *this;
+ SmallVector<Instruction *, 4> NewInsts;
+ for (Instruction *Inst : Insts) {
+ Instruction *Next = Inst->getNextNonDebugInstruction();
+ // Already at end of block.
+ if (!Next) {
+ Fail = true;
+ return *this;
+ }
+ NewInsts.push_back(Next);
+ }
+ if (NewInsts.empty())
+ Fail = true;
+ else
+ Insts = NewInsts;
+ return *this;
+ }
+};
+
+} // 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 6651281ff2d01..5b180c2390bfa 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
diff erent 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 27b7ec4629a26..44cefcd9745e6 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>
@@ -2499,7 +2500,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]
@@ -2529,7 +2530,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