Skip to content

[CodeGen] Use SmallVector for MBB preds/succs #101948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

aengelke
Copy link
Contributor

@aengelke aengelke commented Aug 5, 2024

Avoid extra heap allocations for typical predecessor/successor counts.

c-t-t -0.14% stage2-O0-g

Avoid extra heap allocations for typical predecessor/successor counts.
@llvmbot llvmbot added backend:Hexagon llvm:analysis Includes value tracking, cost tables and constant folding labels Aug 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-hexagon

Author: Alexis Engelke (aengelke)

Changes

Avoid extra heap allocations for typical predecessor/successor counts.

c-t-t -0.14% stage2-O0-g


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

3 Files Affected:

  • (modified) llvm/include/llvm/Analysis/RegionInfoImpl.h (+1-1)
  • (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+12-10)
  • (modified) llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp (+1-1)
diff --git a/llvm/include/llvm/Analysis/RegionInfoImpl.h b/llvm/include/llvm/Analysis/RegionInfoImpl.h
index c5e8821858fd2..2ea432ea95d37 100644
--- a/llvm/include/llvm/Analysis/RegionInfoImpl.h
+++ b/llvm/include/llvm/Analysis/RegionInfoImpl.h
@@ -814,7 +814,7 @@ RegionInfoBase<Tr>::getMaxRegionExit(BlockT *BB) const {
     // Get the single exit of BB.
     if (R && R->getEntry() == BB)
       Exit = R->getExit();
-    else if (++BlockTraits::child_begin(BB) == BlockTraits::child_end(BB))
+    else if (BlockTraits::child_begin(BB) + 1 == BlockTraits::child_end(BB))
       Exit = *BlockTraits::child_begin(BB);
     else // No single exit exists.
       return Exit;
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index b8153fd5d3fb7..5b80827b780b5 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -157,8 +157,8 @@ class MachineBasicBlock
   Instructions Insts;
 
   /// Keep track of the predecessor / successor basic blocks.
-  std::vector<MachineBasicBlock *> Predecessors;
-  std::vector<MachineBasicBlock *> Successors;
+  SmallVector<MachineBasicBlock *, 4> Predecessors;
+  SmallVector<MachineBasicBlock *, 2> Successors;
 
   /// Keep track of the probabilities to the successors. This vector has the
   /// same order as Successors, or it is empty if we don't use it (disable
@@ -387,18 +387,20 @@ class MachineBasicBlock
   }
 
   // Machine-CFG iterators
-  using pred_iterator = std::vector<MachineBasicBlock *>::iterator;
-  using const_pred_iterator = std::vector<MachineBasicBlock *>::const_iterator;
-  using succ_iterator = std::vector<MachineBasicBlock *>::iterator;
-  using const_succ_iterator = std::vector<MachineBasicBlock *>::const_iterator;
+  using pred_iterator = SmallVectorImpl<MachineBasicBlock *>::iterator;
+  using const_pred_iterator =
+      SmallVectorImpl<MachineBasicBlock *>::const_iterator;
+  using succ_iterator = SmallVectorImpl<MachineBasicBlock *>::iterator;
+  using const_succ_iterator =
+      SmallVectorImpl<MachineBasicBlock *>::const_iterator;
   using pred_reverse_iterator =
-      std::vector<MachineBasicBlock *>::reverse_iterator;
+      SmallVectorImpl<MachineBasicBlock *>::reverse_iterator;
   using const_pred_reverse_iterator =
-      std::vector<MachineBasicBlock *>::const_reverse_iterator;
+      SmallVectorImpl<MachineBasicBlock *>::const_reverse_iterator;
   using succ_reverse_iterator =
-      std::vector<MachineBasicBlock *>::reverse_iterator;
+      SmallVectorImpl<MachineBasicBlock *>::reverse_iterator;
   using const_succ_reverse_iterator =
-      std::vector<MachineBasicBlock *>::const_reverse_iterator;
+      SmallVectorImpl<MachineBasicBlock *>::const_reverse_iterator;
   pred_iterator        pred_begin()       { return Predecessors.begin(); }
   const_pred_iterator  pred_begin() const { return Predecessors.begin(); }
   pred_iterator        pred_end()         { return Predecessors.end();   }
diff --git a/llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp b/llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp
index e9d95c6e89db4..a43042a303093 100644
--- a/llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp
@@ -249,7 +249,7 @@ void HexagonCopyHoisting::moveCopyInstr(MachineBasicBlock *DestBB,
   DestBB->splice(FirstTI, MI->getParent(), MI);
 
   addMItoCopyList(MI);
-  for (auto I = ++(DestBB->succ_begin()), E = DestBB->succ_end(); I != E; ++I) {
+  for (auto I = DestBB->succ_begin() + 1, E = DestBB->succ_end(); I != E; ++I) {
     MachineBasicBlock *SuccBB = *I;
     auto &BBCopyInst = CopyMIList[SuccBB->getNumber()];
     MachineInstr *SuccMI = BBCopyInst[Key];

@nikic
Copy link
Contributor

nikic commented Aug 5, 2024

sccache C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\CodeGen -IC:\ws\src\llvm\lib\CodeGen -Iinclude -IC:\ws\src\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /O2 /Ob2  -MD  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Folib\CodeGen\CMakeFiles\LLVMCodeGen.dir\MIRSampleProfile.cpp.obj /Fdlib\CodeGen\CMakeFiles\LLVMCodeGen.dir\LLVMCodeGen.pdb /FS -c C:\ws\src\llvm\lib\CodeGen\MIRSampleProfile.cpp
C:\ws\src\llvm\lib\CodeGen\MIRSampleProfile.cpp(136): error C2440: 'return': cannot convert from 'llvm::iterator_range<llvm::MachineBasicBlock::pred_iterator>' to 'llvm::iterator_range<std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>>'
        with
        [
            _Ty=llvm::MachineBasicBlock *
        ]
C:\ws\src\llvm\lib\CodeGen\MIRSampleProfile.cpp(136): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
C:\ws\src\llvm\lib\CodeGen\MIRSampleProfile.cpp(139): error C2440: 'return': cannot convert from 'llvm::iterator_range<llvm::MachineBasicBlock::pred_iterator>' to 'llvm::iterator_range<std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>>'
        with
        [
            _Ty=llvm::MachineBasicBlock *
        ]
C:\ws\src\llvm\lib\CodeGen\MIRSampleProfile.cpp(139): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

@@ -814,7 +814,7 @@ RegionInfoBase<Tr>::getMaxRegionExit(BlockT *BB) const {
// Get the single exit of BB.
if (R && R->getEntry() == BB)
Exit = R->getExit();
else if (++BlockTraits::child_begin(BB) == BlockTraits::child_end(BB))
else if (BlockTraits::child_begin(BB) + 1 == BlockTraits::child_end(BB))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use std::next() to make this independent of implementation details?

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this wasn't already the case

@llvmbot llvmbot added the PGO Profile Guided Optimizations label Aug 6, 2024
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aengelke aengelke merged commit 4c23c1b into llvm:main Aug 6, 2024
8 checks passed
@aengelke aengelke deleted the mbbsmallvector branch August 6, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:Hexagon llvm:analysis Includes value tracking, cost tables and constant folding llvm:codegen PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants