[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

diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 3446903..970eec5 100644 (file)
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -656,7 +656,7 @@ public:
   /// 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 @@ public:
   /// 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 @@ public:
   /// 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 @@ public:
 /// 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 c2a4bf8..6deb2dc 100644 (file)
--- 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 37be2ea..aa489d0 100644 (file)
--- 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 f31eae9..cf22230 100644 (file)
--- 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