[SSAUpdater] Avoid un-necessary SmallVector stores

[jmorse]

The default template for the generic IDF calculator fetching block-children will, by default:

• Fetch the children range from the relevant GraphTraits,
• Store all child nodes in a SmallVector,
• Return that SmallVector into the IDF calculator.

The only place this SmallVector is used is in a for-range loop... thus there's no reason why we can't just iterate over the child range from GraphTraits, instead of storing all the nodes locally then iterating over the local copy. (If the children of a node change during IDF calculation, everything is broken anyway).

This yields a 0.14% debug-info build performance improvement on the compile time tracker, as InstrRefBasedLDV uses the SSA updater intensively on all functions.

http://llvm-compile-time-tracker.com/compare.php?from=a9f1d377013b1d208ac58f6f75548f1f24d174d2&to=e73fd09de063a113a387c09b561cb3ccfc50e35b&stat=instructions:u

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Jeremy Morse (jmorse)

Changes

The default template for the generic IDF calculator fetching block-children will, by default:

• Fetch the children range from the relevant GraphTraits,
• Store all child nodes in a SmallVector,
• Return that SmallVector into the IDF calculator.

The only place this SmallVector is used is in a for-range loop... thus there's no reason why we can't just iterate over the child range from GraphTraits, instead of storing all the nodes locally then iterating over the local copy. (If the children of a node change during IDF calculation, everything is broken anyway).

This yields a 0.14% debug-info build performance improvement on the compile time tracker, as InstrRefBasedLDV uses the SSA updater intensively on all functions.

http://llvm-compile-time-tracker.com/compare.php?from=a9f1d377013b1d208ac58f6f75548f1f24d174d2&to=e73fd09de063a113a387c09b561cb3ccfc50e35b&stat=instructions:u

---

Full diff: https://github.com/llvm/llvm-project/pull/97820.diff

1 Files Affected:

• (modified) llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h (+6-5)

diff --git a/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h b/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
index 0dc58e37c821d..a7a8b2cecaecc 100644
--- a/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
+++ b/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
@@ -23,6 +23,7 @@
 #ifndef LLVM_SUPPORT_GENERICITERATEDDOMINANCEFRONTIER_H
 #define LLVM_SUPPORT_GENERICITERATEDDOMINANCEFRONTIER_H
 
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/GenericDomTree.h"
@@ -37,9 +38,10 @@ namespace IDFCalculatorDetail {
 /// successors.
 template <class NodeTy, bool IsPostDom> struct ChildrenGetterTy {
   using NodeRef = typename GraphTraits<NodeTy *>::NodeRef;
-  using ChildrenTy = SmallVector<NodeRef, 8>;
+  using ChildIteratorType = typename GraphTraits<NodeTy *>::ChildIteratorType;
+  using range = iterator_range<ChildIteratorType>;
 
-  ChildrenTy get(const NodeRef &N);
+  range get(const NodeRef &N);
 };
 
 } // end of namespace IDFCalculatorDetail
@@ -115,13 +117,12 @@ template <class NodeTy, bool IsPostDom> class IDFCalculatorBase {
 namespace IDFCalculatorDetail {
 
 template <class NodeTy, bool IsPostDom>
-typename ChildrenGetterTy<NodeTy, IsPostDom>::ChildrenTy
+typename ChildrenGetterTy<NodeTy, IsPostDom>::range
 ChildrenGetterTy<NodeTy, IsPostDom>::get(const NodeRef &N) {
   using OrderedNodeTy =
       typename IDFCalculatorBase<NodeTy, IsPostDom>::OrderedNodeTy;
 
-  auto Children = children<OrderedNodeTy>(N);
-  return {Children.begin(), Children.end()};
+  return children<OrderedNodeTy>(N);
 }
 
 } // end of namespace IDFCalculatorDetail

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[OCHyams]

LGTM

Digging around to understand the setup - the IDFCalculator (non-base version) in IteratedDominanceFrontier.h still uses a vector based solution through the namespace IDFCalculatorDetail (and it needs to because it modifies the vector). This IDFCalculatorBase change makes sense to me - AFAICT only affects InstrRefBasedLDV? Either way, given the returned collection is only used in one place (in IDFCalculatorBase::calculate) and is just iterated over this change SGTM.