[llvm] 48a6df3 - Reapply "[Utils] Consolidate `LockstepReverseIterator` into own header (NFC)"
Antonio Frighetto via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 22 02:22:00 PST 2025
Author: Antonio Frighetto
Date: 2025-02-22T11:21:36+01:00
New Revision: 48a6df36040d59a4807b0fa9ac46c343900e5225
URL: https://github.com/llvm/llvm-project/commit/48a6df36040d59a4807b0fa9ac46c343900e5225
DIFF: https://github.com/llvm/llvm-project/commit/48a6df36040d59a4807b0fa9ac46c343900e5225.diff
LOG: Reapply "[Utils] Consolidate `LockstepReverseIterator` into own header (NFC)"
Common code has been unified and generalized.
Original commit: 123dca9b56e1359d8ec7771ea3bd0afd4b1ea6af
Previously reverted due to accidentally merged incompletely. The issue has
been addressed by restoring missing code.
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