From 1812319292e0041e7e32ad7b61d7252a450b95c2 Mon Sep 17 00:00:00 2001 From: Hongtao Yu Date: Mon, 19 Apr 2021 08:51:57 -0700 Subject: [PATCH] [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 --- llvm/include/llvm/CodeGen/MachineBasicBlock.h | 32 +++++++++++++++------------ llvm/lib/CodeGen/BranchFolding.cpp | 10 ++++----- llvm/lib/CodeGen/IfConversion.cpp | 16 +++++++------- llvm/lib/CodeGen/MachineBasicBlock.cpp | 4 +++- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h index 3446903..970eec5 100644 --- 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(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(this)->getLastNonDebugInstr( SkipPseudoOp); } @@ -1113,7 +1117,7 @@ public: /// const_instr_iterator} and the respective reverse iterators. template 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 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 -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 -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 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 --- 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 --- 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 --- 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 -- 2.7.4