Remove the unused IntervalPartition analysis pass #88133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the old legacy PM "intervals" analysis pass (aka
IntervalPartition). It also removes the associated Interval and
IntervalIterator help classes.
Reasons for removal:
1) The pass is not used by llvm-project (not even being tested by
any regression tests).
2) Pass has not been ported to new pass manager, which at least
indicates that it isn't used by the middle-end.
3) ASan reports heap-use-after-free on
++I; // After the first one...
even if false is passed to intervals_begin. Not sure if that is
a false positive, but it makes the code a bit less trustworthy.
|
@llvm/pr-subscribers-llvm-analysis Author: Björn Pettersson (bjope) ChangesThis removes the old legacy PM "intervals" analysis pass (aka IntervalPartition). It also removes the associated Interval and IntervalIterator help classes. Reasons for removal:
Patch is 30.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88133.diff 11 Files Affected:
diff --git a/llvm/include/llvm/Analysis/Interval.h b/llvm/include/llvm/Analysis/Interval.h
deleted file mode 100644
index 9afe659d00dd3d..00000000000000
--- a/llvm/include/llvm/Analysis/Interval.h
+++ /dev/null
@@ -1,138 +0,0 @@
-//===- llvm/Analysis/Interval.h - Interval Class Declaration ----*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains the declaration of the Interval class, which
-// represents a set of CFG nodes and is a portion of an interval partition.
-//
-// Intervals have some interesting and useful properties, including the
-// following:
-// 1. The header node of an interval dominates all of the elements of the
-// interval
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_ANALYSIS_INTERVAL_H
-#define LLVM_ANALYSIS_INTERVAL_H
-
-#include "llvm/ADT/GraphTraits.h"
-#include <vector>
-
-namespace llvm {
-
-class BasicBlock;
-class raw_ostream;
-
-//===----------------------------------------------------------------------===//
-//
-/// Interval Class - An Interval is a set of nodes defined such that every node
-/// in the interval has all of its predecessors in the interval (except for the
-/// header)
-///
-class Interval {
- /// HeaderNode - The header BasicBlock, which dominates all BasicBlocks in this
- /// interval. Also, any loops in this interval must go through the HeaderNode.
- ///
- BasicBlock *HeaderNode;
-
-public:
- using succ_iterator = std::vector<BasicBlock*>::iterator;
- using pred_iterator = std::vector<BasicBlock*>::iterator;
- using node_iterator = std::vector<BasicBlock*>::iterator;
-
- inline Interval(BasicBlock *Header) : HeaderNode(Header) {
- Nodes.push_back(Header);
- }
-
- inline BasicBlock *getHeaderNode() const { return HeaderNode; }
-
- /// Nodes - The basic blocks in this interval.
- std::vector<BasicBlock*> Nodes;
-
- /// Successors - List of BasicBlocks that are reachable directly from nodes in
- /// this interval, but are not in the interval themselves.
- /// These nodes necessarily must be header nodes for other intervals.
- std::vector<BasicBlock*> Successors;
-
- /// Predecessors - List of BasicBlocks that have this Interval's header block
- /// as one of their successors.
- std::vector<BasicBlock*> Predecessors;
-
- /// contains - Find out if a basic block is in this interval
- inline bool contains(BasicBlock *BB) const {
- for (BasicBlock *Node : Nodes)
- if (Node == BB)
- return true;
- return false;
- // I don't want the dependency on <algorithm>
- //return find(Nodes.begin(), Nodes.end(), BB) != Nodes.end();
- }
-
- /// isSuccessor - find out if a basic block is a successor of this Interval
- inline bool isSuccessor(BasicBlock *BB) const {
- for (BasicBlock *Successor : Successors)
- if (Successor == BB)
- return true;
- return false;
- // I don't want the dependency on <algorithm>
- //return find(Successors.begin(), Successors.end(), BB) != Successors.end();
- }
-
- /// Equality operator. It is only valid to compare two intervals from the
- /// same partition, because of this, all we have to check is the header node
- /// for equality.
- inline bool operator==(const Interval &I) const {
- return HeaderNode == I.HeaderNode;
- }
-
- /// print - Show contents in human readable format...
- void print(raw_ostream &O) const;
-};
-
-/// succ_begin/succ_end - define methods so that Intervals may be used
-/// just like BasicBlocks can with the succ_* functions, and *::succ_iterator.
-///
-inline Interval::succ_iterator succ_begin(Interval *I) {
- return I->Successors.begin();
-}
-inline Interval::succ_iterator succ_end(Interval *I) {
- return I->Successors.end();
-}
-
-/// pred_begin/pred_end - define methods so that Intervals may be used
-/// just like BasicBlocks can with the pred_* functions, and *::pred_iterator.
-///
-inline Interval::pred_iterator pred_begin(Interval *I) {
- return I->Predecessors.begin();
-}
-inline Interval::pred_iterator pred_end(Interval *I) {
- return I->Predecessors.end();
-}
-
-template <> struct GraphTraits<Interval*> {
- using NodeRef = Interval *;
- using ChildIteratorType = Interval::succ_iterator;
-
- static NodeRef getEntryNode(Interval *I) { return I; }
-
- /// nodes_iterator/begin/end - Allow iteration over all nodes in the graph
- static ChildIteratorType child_begin(NodeRef N) { return succ_begin(N); }
- static ChildIteratorType child_end(NodeRef N) { return succ_end(N); }
-};
-
-template <> struct GraphTraits<Inverse<Interval*>> {
- using NodeRef = Interval *;
- using ChildIteratorType = Interval::pred_iterator;
-
- static NodeRef getEntryNode(Inverse<Interval *> G) { return G.Graph; }
- static ChildIteratorType child_begin(NodeRef N) { return pred_begin(N); }
- static ChildIteratorType child_end(NodeRef N) { return pred_end(N); }
-};
-
-} // end namespace llvm
-
-#endif // LLVM_ANALYSIS_INTERVAL_H
diff --git a/llvm/include/llvm/Analysis/IntervalIterator.h b/llvm/include/llvm/Analysis/IntervalIterator.h
deleted file mode 100644
index 30e91f1734b655..00000000000000
--- a/llvm/include/llvm/Analysis/IntervalIterator.h
+++ /dev/null
@@ -1,264 +0,0 @@
-//===- IntervalIterator.h - Interval Iterator Declaration -------*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// This file defines an iterator that enumerates the intervals in a control flow
-// graph of some sort. This iterator is parametric, allowing iterator over the
-// following types of graphs:
-//
-// 1. A Function* object, composed of BasicBlock nodes.
-// 2. An IntervalPartition& object, composed of Interval nodes.
-//
-// This iterator is defined to walk the control flow graph, returning intervals
-// in depth first order. These intervals are completely filled in except for
-// the predecessor fields (the successor information is filled in however).
-//
-// By default, the intervals created by this iterator are deleted after they
-// are no longer any use to the iterator. This behavior can be changed by
-// passing a false value into the intervals_begin() function. This causes the
-// IOwnMem member to be set, and the intervals to not be deleted.
-//
-// It is only safe to use this if all of the intervals are deleted by the caller
-// and all of the intervals are processed. However, the user of the iterator is
-// not allowed to modify or delete the intervals until after the iterator has
-// been used completely. The IntervalPartition class uses this functionality.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_ANALYSIS_INTERVALITERATOR_H
-#define LLVM_ANALYSIS_INTERVALITERATOR_H
-
-#include "llvm/ADT/GraphTraits.h"
-#include "llvm/Analysis/Interval.h"
-#include "llvm/Analysis/IntervalPartition.h"
-#include "llvm/IR/CFG.h"
-#include <algorithm>
-#include <cassert>
-#include <iterator>
-#include <set>
-#include <utility>
-#include <vector>
-
-namespace llvm {
-
-class BasicBlock;
-class Function;
-
-// getNodeHeader - Given a source graph node and the source graph, return the
-// BasicBlock that is the header node. This is the opposite of
-// getSourceGraphNode.
-inline BasicBlock *getNodeHeader(BasicBlock *BB) { return BB; }
-inline BasicBlock *getNodeHeader(Interval *I) { return I->getHeaderNode(); }
-
-// getSourceGraphNode - Given a BasicBlock and the source graph, return the
-// source graph node that corresponds to the BasicBlock. This is the opposite
-// of getNodeHeader.
-inline BasicBlock *getSourceGraphNode(Function *, BasicBlock *BB) {
- return BB;
-}
-inline Interval *getSourceGraphNode(IntervalPartition *IP, BasicBlock *BB) {
- return IP->getBlockInterval(BB);
-}
-
-// addNodeToInterval - This method exists to assist the generic ProcessNode
-// with the task of adding a node to the new interval, depending on the
-// type of the source node. In the case of a CFG source graph (BasicBlock
-// case), the BasicBlock itself is added to the interval.
-inline void addNodeToInterval(Interval *Int, BasicBlock *BB) {
- Int->Nodes.push_back(BB);
-}
-
-// addNodeToInterval - This method exists to assist the generic ProcessNode
-// with the task of adding a node to the new interval, depending on the
-// type of the source node. In the case of a CFG source graph (BasicBlock
-// case), the BasicBlock itself is added to the interval. In the case of
-// an IntervalPartition source graph (Interval case), all of the member
-// BasicBlocks are added to the interval.
-inline void addNodeToInterval(Interval *Int, Interval *I) {
- // Add all of the nodes in I as new nodes in Int.
- llvm::append_range(Int->Nodes, I->Nodes);
-}
-
-template<class NodeTy, class OrigContainer_t, class GT = GraphTraits<NodeTy *>,
- class IGT = GraphTraits<Inverse<NodeTy *>>>
-class IntervalIterator {
- std::vector<std::pair<Interval *, typename Interval::succ_iterator>> IntStack;
- std::set<BasicBlock *> Visited;
- OrigContainer_t *OrigContainer;
- bool IOwnMem; // If True, delete intervals when done with them
- // See file header for conditions of use
-
-public:
- using iterator_category = std::forward_iterator_tag;
-
- IntervalIterator() = default; // End iterator, empty stack
-
- IntervalIterator(Function *M, bool OwnMemory) : IOwnMem(OwnMemory) {
- OrigContainer = M;
- if (!ProcessInterval(&M->front())) {
- llvm_unreachable("ProcessInterval should never fail for first interval!");
- }
- }
-
- IntervalIterator(IntervalIterator &&x)
- : IntStack(std::move(x.IntStack)), Visited(std::move(x.Visited)),
- OrigContainer(x.OrigContainer), IOwnMem(x.IOwnMem) {
- x.IOwnMem = false;
- }
-
- IntervalIterator(IntervalPartition &IP, bool OwnMemory) : IOwnMem(OwnMemory) {
- OrigContainer = &IP;
- if (!ProcessInterval(IP.getRootInterval())) {
- llvm_unreachable("ProcessInterval should never fail for first interval!");
- }
- }
-
- ~IntervalIterator() {
- if (IOwnMem)
- while (!IntStack.empty()) {
- delete operator*();
- IntStack.pop_back();
- }
- }
-
- bool operator==(const IntervalIterator &x) const {
- return IntStack == x.IntStack;
- }
- bool operator!=(const IntervalIterator &x) const { return !(*this == x); }
-
- const Interval *operator*() const { return IntStack.back().first; }
- Interval *operator*() { return IntStack.back().first; }
- const Interval *operator->() const { return operator*(); }
- Interval *operator->() { return operator*(); }
-
- IntervalIterator &operator++() { // Preincrement
- assert(!IntStack.empty() && "Attempting to use interval iterator at end!");
- do {
- // All of the intervals on the stack have been visited. Try visiting
- // their successors now.
- Interval::succ_iterator &SuccIt = IntStack.back().second,
- EndIt = succ_end(IntStack.back().first);
- while (SuccIt != EndIt) { // Loop over all interval succs
- bool Done = ProcessInterval(getSourceGraphNode(OrigContainer, *SuccIt));
- ++SuccIt; // Increment iterator
- if (Done) return *this; // Found a new interval! Use it!
- }
-
- // Free interval memory... if necessary
- if (IOwnMem) delete IntStack.back().first;
-
- // We ran out of successors for this interval... pop off the stack
- IntStack.pop_back();
- } while (!IntStack.empty());
-
- return *this;
- }
-
- IntervalIterator operator++(int) { // Postincrement
- IntervalIterator tmp = *this;
- ++*this;
- return tmp;
- }
-
-private:
- // ProcessInterval - This method is used during the construction of the
- // interval graph. It walks through the source graph, recursively creating
- // an interval per invocation until the entire graph is covered. This uses
- // the ProcessNode method to add all of the nodes to the interval.
- //
- // This method is templated because it may operate on two different source
- // graphs: a basic block graph, or a preexisting interval graph.
- bool ProcessInterval(NodeTy *Node) {
- BasicBlock *Header = getNodeHeader(Node);
- if (!Visited.insert(Header).second)
- return false;
-
- Interval *Int = new Interval(Header);
-
- // Check all of our successors to see if they are in the interval...
- for (typename GT::ChildIteratorType I = GT::child_begin(Node),
- E = GT::child_end(Node); I != E; ++I)
- ProcessNode(Int, getSourceGraphNode(OrigContainer, *I));
-
- IntStack.push_back(std::make_pair(Int, succ_begin(Int)));
- return true;
- }
-
- // ProcessNode - This method is called by ProcessInterval to add nodes to the
- // interval being constructed, and it is also called recursively as it walks
- // the source graph. A node is added to the current interval only if all of
- // its predecessors are already in the graph. This also takes care of keeping
- // the successor set of an interval up to date.
- //
- // This method is templated because it may operate on two different source
- // graphs: a basic block graph, or a preexisting interval graph.
- void ProcessNode(Interval *Int, NodeTy *Node) {
- assert(Int && "Null interval == bad!");
- assert(Node && "Null Node == bad!");
-
- BasicBlock *NodeHeader = getNodeHeader(Node);
-
- if (Visited.count(NodeHeader)) { // Node already been visited?
- if (Int->contains(NodeHeader)) { // Already in this interval...
- return;
- } else { // In other interval, add as successor
- if (!Int->isSuccessor(NodeHeader)) // Add only if not already in set
- Int->Successors.push_back(NodeHeader);
- }
- } else { // Otherwise, not in interval yet
- for (typename IGT::ChildIteratorType I = IGT::child_begin(Node),
- E = IGT::child_end(Node); I != E; ++I) {
- if (!Int->contains(*I)) { // If pred not in interval, we can't be
- if (!Int->isSuccessor(NodeHeader)) // Add only if not already in set
- Int->Successors.push_back(NodeHeader);
- return; // See you later
- }
- }
-
- // If we get here, then all of the predecessors of BB are in the interval
- // already. In this case, we must add BB to the interval!
- addNodeToInterval(Int, Node);
- Visited.insert(NodeHeader); // The node has now been visited!
-
- if (Int->isSuccessor(NodeHeader)) {
- // If we were in the successor list from before... remove from succ list
- llvm::erase(Int->Successors, NodeHeader);
- }
-
- // Now that we have discovered that Node is in the interval, perhaps some
- // of its successors are as well?
- for (typename GT::ChildIteratorType It = GT::child_begin(Node),
- End = GT::child_end(Node); It != End; ++It)
- ProcessNode(Int, getSourceGraphNode(OrigContainer, *It));
- }
- }
-};
-
-using function_interval_iterator = IntervalIterator<BasicBlock, Function>;
-using interval_part_interval_iterator =
- IntervalIterator<Interval, IntervalPartition>;
-
-inline function_interval_iterator intervals_begin(Function *F,
- bool DeleteInts = true) {
- return function_interval_iterator(F, DeleteInts);
-}
-inline function_interval_iterator intervals_end(Function *) {
- return function_interval_iterator();
-}
-
-inline interval_part_interval_iterator
- intervals_begin(IntervalPartition &IP, bool DeleteIntervals = true) {
- return interval_part_interval_iterator(IP, DeleteIntervals);
-}
-
-inline interval_part_interval_iterator intervals_end(IntervalPartition &IP) {
- return interval_part_interval_iterator();
-}
-
-} // end namespace llvm
-
-#endif // LLVM_ANALYSIS_INTERVALITERATOR_H
diff --git a/llvm/include/llvm/Analysis/IntervalPartition.h b/llvm/include/llvm/Analysis/IntervalPartition.h
deleted file mode 100644
index 66a99fb15bfb9f..00000000000000
--- a/llvm/include/llvm/Analysis/IntervalPartition.h
+++ /dev/null
@@ -1,108 +0,0 @@
-//===- IntervalPartition.h - Interval partition Calculation -----*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains the declaration of the IntervalPartition class, which
-// calculates and represents the interval partition of a function, or a
-// preexisting interval partition.
-//
-// In this way, the interval partition may be used to reduce a flow graph down
-// to its degenerate single node interval partition (unless it is irreducible).
-//
-// TODO: The IntervalPartition class should take a bool parameter that tells
-// whether it should add the "tails" of an interval to an interval itself or if
-// they should be represented as distinct intervals.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_ANALYSIS_INTERVALPARTITION_H
-#define LLVM_ANALYSIS_INTERVALPARTITION_H
-
-#include "llvm/Pass.h"
-#include <map>
-#include <vector>
-
-namespace llvm {
-
-class BasicBlock;
-class Interval;
-
-//===----------------------------------------------------------------------===//
-//
-// IntervalPartition - This class builds and holds an "interval partition" for
-// a function. This partition divides the control flow graph into a set of
-// maximal intervals, as defined with the properties above. Intuitively, an
-// interval is a (possibly nonexistent) loop with a "tail" of non-looping
-// nodes following it.
-//
-class IntervalPartition : public FunctionPass {
- using IntervalMapTy = std::map<BasicBlock *, Interval *>;
- IntervalMapTy IntervalMap;
-
- using IntervalListTy = std::vector<Interval *>;
- Interval *RootInterval = nullptr;
- std::vector<Interval *> Intervals;
-
-public:
- static char ID; // Pass identification, replacement for typeid
-
- IntervalPartition();
-
- // run - Calculate the interval partition for this function
- bool runOnFunction(Function &F) override;
-
- // IntervalPartition ctor - Build a reduced interval partition from an
- // existing interval graph. This takes an additional boolean parameter to
- // distinguish it from a copy constructor. Always pass in false for now.
- IntervalPartition(IntervalPartition &I, bool);
-
- // print - Show contents in human readable format...
- void print(raw_ostream &O, const Module* = nullptr) const override;
-
- // getRootInterval() - Return the root interval that contains the starting
- // block of the function.
- inline Interval *getRootInterval() { return RootInterval; }
-
- // isDegeneratePartition() - Returns true if the interval partition contains
- // a single interval, and thus cannot be simplified anymore.
- bool isDegeneratePartition() { return Intervals.size() == 1; }
-
- // TODO: isIrreducible - look for triangle graph.
-
- // getBlockInterval - Return the interval that a basic block exists in.
- inline Interval *getBlockInterval(BasicBlock *BB) {
- IntervalMapTy::iterator I = IntervalMap.find(BB);
- return I != IntervalMap.end() ? I->second : nullptr;
- }
-
- // getAnalysisUsage - Implement the Pass API
- void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.setPreservesAll();
- }
-
- // Interface to Intervals vector...
- const std::vector<Interval*> &getIntervals() const { return Intervals; }
-
- // releaseMemory - Reset state back to before function was analyzed
- void releaseMemory() override;
-
-private:
- // addIntervalToPartition - Add an interval to the internal list of intervals,
- // and then add mappings from all of the basic blocks in the interval to the
- // interval itself (i...
[truncated]
|
aeubanks
approved these changes
Apr 9, 2024
|
Thanks for the cleanup! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the old legacy PM "intervals" analysis pass (aka IntervalPartition). It also removes the associated Interval and IntervalIterator help classes.
Reasons for removal:
any regression tests).
indicates that it isn't used by the middle-end.
++I; // After the first one...
even if false is passed to intervals_begin. Not sure if that is
a false positive, but it makes the code a bit less trustworthy.