[llvm] 236fa50 - Revert "[Utils] Consolidate `LockstepReverseIterator` into own header (NFC) (#116657)"
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 21 13:01:48 PST 2025
Author: Florian Hahn
Date: 2025-02-21T21:00:28Z
New Revision: 236fa506d4f166882b63029915333d9599014644
URL: https://github.com/llvm/llvm-project/commit/236fa506d4f166882b63029915333d9599014644
DIFF: https://github.com/llvm/llvm-project/commit/236fa506d4f166882b63029915333d9599014644.diff
LOG: Revert "[Utils] Consolidate `LockstepReverseIterator` into own header (NFC) (#116657)"
This reverts commit 123dca9b56e1359d8ec7771ea3bd0afd4b1ea6af.
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);
| ^~~~~~~~~~~~~~~~~~~~~~~
Added:
Modified:
llvm/lib/Transforms/Scalar/GVNSink.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Removed:
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
deleted file mode 100644
index cc9d717054528..0000000000000
--- a/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
+++ /dev/null
@@ -1,155 +0,0 @@
-//===- 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 5b180c2390bfa..6651281ff2d01 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -65,7 +65,6 @@
#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>
@@ -97,6 +96,87 @@ 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
@@ -554,11 +634,9 @@ 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<false> &LRI,
- unsigned &InstNum, unsigned &MemoryInstNum,
- ModelledPHISet &NeededPHIs,
- SmallPtrSetImpl<Value *> &PHIContents);
+ std::optional<SinkingInstructionCandidate> analyzeInstructionForSinking(
+ LockstepReverseIterator &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,
@@ -597,7 +675,7 @@ class GVNSink {
};
std::optional<SinkingInstructionCandidate>
-GVNSink::analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
+GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
unsigned &InstNum,
unsigned &MemoryInstNum,
ModelledPHISet &NeededPHIs,
@@ -749,7 +827,7 @@ unsigned GVNSink::sinkBB(BasicBlock *BBEnd) {
return BB->getTerminator()->getNumSuccessors() != 1;
});
- LockstepReverseIterator<false> LRI(Preds);
+ LockstepReverseIterator 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 44cefcd9745e6..27b7ec4629a26 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -74,7 +74,6 @@
#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>
@@ -2500,7 +2499,7 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
int ScanIdx = 0;
SmallPtrSet<Value*,4> InstructionsToSink;
- LockstepReverseIterator<true> LRI(UnconditionalPreds);
+ LockstepReverseIterator LRI(UnconditionalPreds);
while (LRI.isValid() &&
canSinkInstructions(*LRI, PHIOperands)) {
LLVM_DEBUG(dbgs() << "SINK: instruction can be sunk: " << *(*LRI)[0]
@@ -2530,7 +2529,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<true> &LRI) {
+ auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) {
unsigned NumPHIInsts = 0;
for (Use &U : (*LRI)[0]->operands()) {
auto It = PHIOperands.find(&U);
More information about the llvm-commits
mailing list