[llvm] 1812319 - [CSSPGO] Flip SkipPseudoOp to true for MIR APIs.

Hongtao Yu via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 19 17:55:46 PDT 2021


Author: Hongtao Yu
Date: 2021-04-19T17:55:34-07:00
New Revision: 1812319292e0041e7e32ad7b61d7252a450b95c2

URL: https://github.com/llvm/llvm-project/commit/1812319292e0041e7e32ad7b61d7252a450b95c2
DIFF: https://github.com/llvm/llvm-project/commit/1812319292e0041e7e32ad7b61d7252a450b95c2.diff

LOG: [CSSPGO] Flip SkipPseudoOp to true for MIR APIs.

Flipping the default value of SkipPseudoOp to true for those MIR APIs to favor maximum performance. Note that certain spots like branch folding and MIR if-conversion is are disabled for better counts quality. For these two optimizations, this is a no-diff change.

The counts quality with SPEC2017 before/after this change is unchanged.

Reviewed By: wmi

Differential Revision: https://reviews.llvm.org/D100332

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/MachineBasicBlock.h
    llvm/lib/CodeGen/BranchFolding.cpp
    llvm/lib/CodeGen/IfConversion.cpp
    llvm/lib/CodeGen/MachineBasicBlock.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 3446903626a1..970eec567fcc 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -656,7 +656,7 @@ class MachineBasicBlock
   /// Return the first instruction in MBB after I that is not a PHI, label or
   /// debug.  This is the correct point to insert copies at the beginning of a
   /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I);
+  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
 
   /// Returns an iterator to the first terminator instruction of this basic
   /// block. If a terminator does not exist, it returns end().
@@ -681,11 +681,12 @@ class MachineBasicBlock
   /// Therefore, they should be considered as a valid instruction when this
   /// function is called in a context of such optimizations. On the other hand,
   /// \c SkipPseudoOp should be true when it's used in optimizations that
-  /// unlikely hurt profile quality, e.g., without block merging.
-  /// TODO: flip the default value of \c SkipPseudoOp to maximize code quality
-  /// with pseudo probes.
-  iterator getFirstNonDebugInstr(bool SkipPseudoOp = false);
-  const_iterator getFirstNonDebugInstr(bool SkipPseudoOp = false) const {
+  /// unlikely hurt profile quality, e.g., without block merging. The default
+  /// value of \c SkipPseudoOp is set to true to maximize code quality in
+  /// general, with an explict false value passed in in a few places like branch
+  /// folding and if-conversion to favor profile quality.
+  iterator getFirstNonDebugInstr(bool SkipPseudoOp = true);
+  const_iterator getFirstNonDebugInstr(bool SkipPseudoOp = true) const {
     return const_cast<MachineBasicBlock *>(this)->getFirstNonDebugInstr(
         SkipPseudoOp);
   }
@@ -702,9 +703,12 @@ class MachineBasicBlock
   /// Therefore, they should be considered as a valid instruction when this
   /// function is called in a context of such optimizations. On the other hand,
   /// \c SkipPseudoOp should be true when it's used in optimizations that
-  /// unlikely hurt profile quality, e.g., without block merging.
-  iterator getLastNonDebugInstr(bool SkipPseudoOp = false);
-  const_iterator getLastNonDebugInstr(bool SkipPseudoOp = false) const {
+  /// unlikely hurt profile quality, e.g., without block merging. The default
+  /// value of \c SkipPseudoOp is set to true to maximize code quality in
+  /// general, with an explict false value passed in in a few places like branch
+  /// folding and if-conversion to favor profile quality.
+  iterator getLastNonDebugInstr(bool SkipPseudoOp = true);
+  const_iterator getLastNonDebugInstr(bool SkipPseudoOp = true) const {
     return const_cast<MachineBasicBlock *>(this)->getLastNonDebugInstr(
         SkipPseudoOp);
   }
@@ -1113,7 +1117,7 @@ class MachineInstrSpan {
 /// const_instr_iterator} and the respective reverse iterators.
 template <typename IterT>
 inline IterT skipDebugInstructionsForward(IterT It, IterT End,
-                                          bool SkipPseudoOp = false) {
+                                          bool SkipPseudoOp = true) {
   while (It != End &&
          (It->isDebugInstr() || (SkipPseudoOp && It->isPseudoProbe())))
     ++It;
@@ -1126,7 +1130,7 @@ inline IterT skipDebugInstructionsForward(IterT It, IterT End,
 /// const_instr_iterator} and the respective reverse iterators.
 template <class IterT>
 inline IterT skipDebugInstructionsBackward(IterT It, IterT Begin,
-                                           bool SkipPseudoOp = false) {
+                                           bool SkipPseudoOp = true) {
   while (It != Begin &&
          (It->isDebugInstr() || (SkipPseudoOp && It->isPseudoProbe())))
     --It;
@@ -1136,14 +1140,14 @@ inline IterT skipDebugInstructionsBackward(IterT It, IterT Begin,
 /// Increment \p It, then continue incrementing it while it points to a debug
 /// instruction. A replacement for std::next.
 template <typename IterT>
-inline IterT next_nodbg(IterT It, IterT End, bool SkipPseudoOp = false) {
+inline IterT next_nodbg(IterT It, IterT End, bool SkipPseudoOp = true) {
   return skipDebugInstructionsForward(std::next(It), End, SkipPseudoOp);
 }
 
 /// Decrement \p It, then continue decrementing it while it points to a debug
 /// instruction. A replacement for std::prev.
 template <typename IterT>
-inline IterT prev_nodbg(IterT It, IterT Begin, bool SkipPseudoOp = false) {
+inline IterT prev_nodbg(IterT It, IterT Begin, bool SkipPseudoOp = true) {
   return skipDebugInstructionsBackward(std::prev(It), Begin, SkipPseudoOp);
 }
 
@@ -1151,7 +1155,7 @@ inline IterT prev_nodbg(IterT It, IterT Begin, bool SkipPseudoOp = false) {
 /// \p End is reached, skipping any debug instructions.
 template <typename IterT>
 inline auto instructionsWithoutDebug(IterT It, IterT End,
-                                     bool SkipPseudoOp = false) {
+                                     bool SkipPseudoOp = true) {
   return make_filter_range(make_range(It, End), [=](const MachineInstr &MI) {
     return !MI.isDebugInstr() && !(SkipPseudoOp && MI.isPseudoProbe());
   });

diff  --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index c2a4bf8d964e..6deb2dc05483 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -286,7 +286,7 @@ static unsigned HashMachineInstr(const MachineInstr &MI) {
 
 /// HashEndOfMBB - Hash the last instruction in the MBB.
 static unsigned HashEndOfMBB(const MachineBasicBlock &MBB) {
-  MachineBasicBlock::const_iterator I = MBB.getLastNonDebugInstr();
+  MachineBasicBlock::const_iterator I = MBB.getLastNonDebugInstr(false);
   if (I == MBB.end())
     return 0;
 
@@ -566,9 +566,9 @@ ProfitableToMerge(MachineBasicBlock *MBB1, MachineBasicBlock *MBB2,
   // Move the iterators to the beginning of the MBB if we only got debug
   // instructions before the tail. This is to avoid splitting a block when we
   // only got debug instructions before the tail (to be invariant on -g).
-  if (skipDebugInstructionsForward(MBB1->begin(), MBB1->end()) == I1)
+  if (skipDebugInstructionsForward(MBB1->begin(), MBB1->end(), false) == I1)
     I1 = MBB1->begin();
-  if (skipDebugInstructionsForward(MBB2->begin(), MBB2->end()) == I2)
+  if (skipDebugInstructionsForward(MBB2->begin(), MBB2->end(), false) == I2)
     I2 = MBB2->begin();
 
   bool FullBlockTail1 = I1 == MBB1->begin();
@@ -1929,8 +1929,8 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
   MachineBasicBlock::iterator FIE = FBB->end();
   while (TIB != TIE && FIB != FIE) {
     // Skip dbg_value instructions. These do not count.
-    TIB = skipDebugInstructionsForward(TIB, TIE);
-    FIB = skipDebugInstructionsForward(FIB, FIE);
+    TIB = skipDebugInstructionsForward(TIB, TIE, false);
+    FIB = skipDebugInstructionsForward(FIB, FIE, false);
     if (TIB == TIE || FIB == FIE)
       break;
 

diff  --git a/llvm/lib/CodeGen/IfConversion.cpp b/llvm/lib/CodeGen/IfConversion.cpp
index 37be2eabf5fe..aa489d022e16 100644
--- a/llvm/lib/CodeGen/IfConversion.cpp
+++ b/llvm/lib/CodeGen/IfConversion.cpp
@@ -742,8 +742,8 @@ bool IfConverter::CountDuplicatedInstructions(
     bool SkipUnconditionalBranches) const {
   while (TIB != TIE && FIB != FIE) {
     // Skip dbg_value instructions. These do not count.
-    TIB = skipDebugInstructionsForward(TIB, TIE);
-    FIB = skipDebugInstructionsForward(FIB, FIE);
+    TIB = skipDebugInstructionsForward(TIB, TIE, false);
+    FIB = skipDebugInstructionsForward(FIB, FIE, false);
     if (TIB == TIE || FIB == FIE)
       break;
     if (!TIB->isIdenticalTo(*FIB))
@@ -785,8 +785,8 @@ bool IfConverter::CountDuplicatedInstructions(
   while (RTIE != RTIB && RFIE != RFIB) {
     // Skip dbg_value instructions. These do not count.
     // Note that these are reverse iterators going forward.
-    RTIE = skipDebugInstructionsForward(RTIE, RTIB);
-    RFIE = skipDebugInstructionsForward(RFIE, RFIB);
+    RTIE = skipDebugInstructionsForward(RTIE, RTIB, false);
+    RFIE = skipDebugInstructionsForward(RFIE, RFIB, false);
     if (RTIE == RTIB || RFIE == RFIB)
       break;
     if (!RTIE->isIdenticalTo(*RFIE))
@@ -838,8 +838,8 @@ static void verifySameBranchInstructions(
   MachineBasicBlock::reverse_iterator E1 = MBB1->rbegin();
   MachineBasicBlock::reverse_iterator E2 = MBB2->rbegin();
   while (E1 != B1 && E2 != B2) {
-    skipDebugInstructionsForward(E1, B1);
-    skipDebugInstructionsForward(E2, B2);
+    skipDebugInstructionsForward(E1, B1, false);
+    skipDebugInstructionsForward(E2, B2, false);
     if (E1 == B1 && E2 == B2)
       break;
 
@@ -1834,8 +1834,8 @@ bool IfConverter::IfConvertDiamondCommon(
 
   // Remove the duplicated instructions at the beginnings of both paths.
   // Skip dbg_value instructions.
-  MachineBasicBlock::iterator DI1 = MBB1.getFirstNonDebugInstr();
-  MachineBasicBlock::iterator DI2 = MBB2.getFirstNonDebugInstr();
+  MachineBasicBlock::iterator DI1 = MBB1.getFirstNonDebugInstr(false);
+  MachineBasicBlock::iterator DI2 = MBB2.getFirstNonDebugInstr(false);
   BBI1->NonPredSize -= NumDups1;
   BBI2->NonPredSize -= NumDups1;
 

diff  --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index f31eae937235..cf22230b510f 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -221,11 +221,13 @@ MachineBasicBlock::SkipPHIsAndLabels(MachineBasicBlock::iterator I) {
 }
 
 MachineBasicBlock::iterator
-MachineBasicBlock::SkipPHIsLabelsAndDebug(MachineBasicBlock::iterator I) {
+MachineBasicBlock::SkipPHIsLabelsAndDebug(MachineBasicBlock::iterator I,
+                                          bool SkipPseudoOp) {
   const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
 
   iterator E = end();
   while (I != E && (I->isPHI() || I->isPosition() || I->isDebugInstr() ||
+                    (SkipPseudoOp && I->isPseudoProbe()) ||
                     TII->isBasicBlockPrologue(*I)))
     ++I;
   // FIXME: This needs to change if we wish to bundle labels / dbg_values


        


More information about the llvm-commits mailing list