Add optimization bisect opt-in calls for PowerPC passes

[andrew.w.kaylor]

Summary

This patch adds calls to PowerPC-specific passes that can be safely skipped to opt-in to the optimization bisect mechanism.

I selected the passes to be skipped based on the fact that they were not added at CodeGenOpt::None in PPCTargetMachine.cpp. Based on that criteria, I did not add opt-in calls to the following passes, which appear to be required:

PPCBSel
PPCTOCRegDeps
PPCTLSDynamicCall
PPCVSXCopy
PPCVSXFMAMutate

I also chose not to add the skip check to PPCCTRLoopsVerify, but I'm not certain about this one. This pass isn't run at -O0, so it meets the normal criteria for skipping, but since it is a verification function I decided not to add the skip check. If this is verifying that transformations were performed in PPCCTRLoops, which can be skipped, then there is a potential problem. If, on the other hand, it is simply verifying that the IR is legal following PPCCTRLoops then it should be fine to not skip it.

Note that the call to skipFunction() will also check for the "optnone" function attribute, so this can theoretically result in passes being skipped even when optimization bisect is not being done. However, I believe that any pass that can be safely skipped should be skipped for functions with the optnone attribute.

See D19172 for details on the base optimizaton bisect implementation.

[hfinkel]

I selected the passes to be skipped based on the fact that they were not added at CodeGenOpt::None in PPCTargetMachine.cpp. Based on that criteria, I did not add opt-in calls to the following passes, which appear to be required:

PPCBSel
PPCTOCRegDeps
PPCTLSDynamicCall
PPCVSXCopy
PPCVSXFMAMutate

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

I also chose not to add the skip check to PPCCTRLoopsVerify, but I'm not certain about this one. This pass isn't run at -O0, so it meets the normal criteria for skipping, but since it is a verification function I decided not to add the skip check. If this is verifying that transformations were performed in PPCCTRLoops, which can be skipped, then there is a potential problem. If, on the other hand, it is simply verifying that the IR is legal following PPCCTRLoops then it should be fine to not skip it.

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

[andrew.w.kaylor]

In D19554#412767, @hfinkel wrote:

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

It looks to me as if PPCVSXFMAMutate is always added (in PPCPassConfig::addPreRegAlloc), even at -O0. I'm trying to keep the "optnone" function handling and the bisection consistent with -O0. There's a lit test that verifies that nothing is skipped because of the "optnone" function attribute that is otherwise run at -O0. I plan to add a similar check for the opt bisect soon.

It makes sense that PPCVSXFMAMutate would be skippable, but I'd prefer not to add it to the bisection unless it is also being omitted at -O0.

In D19554#412767, @hfinkel wrote:

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

If I add the skip check to PPCCTRLoopsVerify, it will always be skipped if PPCCTRLoops is skipped (because it runs afterward), but it's possible that PPCCTRLoops could be run and PPCCTRLoopsVerify. That should be OK, though right? So should I add the skip check to the verify pass?

[hfinkel]

In D19554#412891, @andrew.w.kaylor wrote:

In D19554#412767, @hfinkel wrote:

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

It looks to me as if PPCVSXFMAMutate is always added (in PPCPassConfig::addPreRegAlloc), even at -O0. I'm trying to keep the "optnone" function handling and the bisection consistent with -O0. There's a lit test that verifies that nothing is skipped because of the "optnone" function attribute that is otherwise run at -O0. I plan to add a similar check for the opt bisect soon.

It makes sense that PPCVSXFMAMutate would be skippable, but I'd prefer not to add it to the bisection unless it is also being omitted at -O0.

Okay; in that case, please change that as well. It can be skipped at -O0 too.

In D19554#412767, @hfinkel wrote:

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

If I add the skip check to PPCCTRLoopsVerify, it will always be skipped if PPCCTRLoops is skipped (because it runs afterward), but it's possible that PPCCTRLoops could be run and PPCCTRLoopsVerify. That should be OK, though right? So should I add the skip check to the verify pass?

No, don't skip it.

[andrew.w.kaylor]

Added code to check of opt level != None before adding the PPCVSXFMAMutate pass
Made the PPCVSXFMAMutate pass skippable

[hfinkel]

LGTM.

lib/Target/PowerPC/PPCBoolRetToInt.cpp

 
  public:
   static char ID;
   PPCBoolRetToInt() : FunctionPass(ID) {
     initializePPCBoolRetToIntPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnFunction(Function &F) {
+    if (skipFunction(F))
+      return false;
+
     PHINodeSet PromotablePHINodes = getPromotablePHINodes(F);
     B2IMap Bool2IntMap;
     bool Changed = false;
     for (auto &BB : F) {
       for (auto &I : BB) {
         if (ReturnInst *R = dyn_cast<ReturnInst>(&I))
           if (F.getReturnType()->isIntegerTy(1))
             Changed |=

lib/Target/PowerPC/PPCCTRLoops.cpp

INITIALIZE_PASS_END(PPCCTRLoopsVerify, "ppc-ctr-loops-verify",
                     "PowerPC CTR Loops Verify", false, false)
 
 FunctionPass *llvm::createPPCCTRLoopsVerify() {
   return new PPCCTRLoopsVerify();
 }
 #endif // NDEBUG
 
 bool PPCCTRLoops::runOnFunction(Function &F) {
+  if (skipFunction(F))
+    return false;
+
   LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
   DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   DL = &F.getParent()->getDataLayout();
   auto *TLIP = getAnalysisIfAvailable<TargetLibraryInfoWrapperPass>();
   LibInfo = TLIP ? &TLIP->getTLI() : nullptr;
   PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);
 

lib/Target/PowerPC/PPCEarlyReturn.cpp

bool processBlock(MachineBasicBlock &ReturnMBB) {
           ReturnMBB.eraseFromParent();
       }
 
       return Changed;
     }
 
 public:
     bool runOnMachineFunction(MachineFunction &MF) override {
+      if (skipFunction(*MF.getFunction()))
+        return false;
+
       TII = MF.getSubtarget().getInstrInfo();
 
       bool Changed = false;
 
       // If the function does not have at least two blocks, then there is
       // nothing to do.
       if (MF.size() < 2)
         return Changed;

lib/Target/PowerPC/PPCLoopPreIncPrep.cpp

if (LoadInst *LMemI = dyn_cast<LoadInst>(MemI)) {
     if (IMemI->getIntrinsicID() == Intrinsic::prefetch)
       return IMemI->getArgOperand(0);
   }
 
   return 0;
 }
 
 bool PPCLoopPreIncPrep::runOnFunction(Function &F) {
+  if (skipFunction(F))
+    return false;
+
   LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
   auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
   DT = DTWP ? &DTWP->getDomTree() : nullptr;
   PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);
 
   bool MadeChange = false;
 

lib/Target/PowerPC/PPCMIPeephole.cpp

private:
 
   // Find the "true" register represented by SrcReg (following chains
   // of copies and subreg_to_reg operations).
   unsigned lookThruCopyLike(unsigned SrcReg);
 
 public:
   // Main entry point for this pass.
   bool runOnMachineFunction(MachineFunction &MF) override {
+    if (skipFunction(*MF.getFunction()))
+      return false;
     initialize(MF);
     return simplifyCode();
   }
 };
 
 // Initialize class variables.
 void PPCMIPeephole::initialize(MachineFunction &MFParm) {
   MF = &MFParm;

lib/Target/PowerPC/PPCQPXLoadSplat.cpp

INITIALIZE_PASS(PPCQPXLoadSplat, "ppc-qpx-load-splat",
                 "PowerPC QPX Load Splat Simplification",
                 false, false)
 
 FunctionPass *llvm::createPPCQPXLoadSplatPass() {
   return new PPCQPXLoadSplat();
 }
 
 bool PPCQPXLoadSplat::runOnMachineFunction(MachineFunction &MF) {
+  if (skipFunction(*MF.getFunction()))
+    return false;
+
   bool MadeChange = false;
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
 
   for (auto MFI = MF.begin(), MFIE = MF.end(); MFI != MFIE; ++MFI) {
     MachineBasicBlock *MBB = &*MFI;
     SmallVector<MachineInstr *, 4> Splats;
 
     for (auto MBBI = MBB->rbegin(); MBBI != MBB->rend(); ++MBBI) {

lib/Target/PowerPC/PPCVSXSwapRemoval.cpp

bool isAnyVecReg(unsigned Reg, bool &Partial) {
     if (isScalarVecReg(Reg))
       Partial = true;
     return isScalarVecReg(Reg) || isVecReg(Reg);
   }
 
 public:
   // Main entry point for this pass.
   bool runOnMachineFunction(MachineFunction &MF) override {
+    if (skipFunction(*MF.getFunction()))
+      return false;
+
     // If we don't have VSX on the subtarget, don't do anything.
     const PPCSubtarget &STI = MF.getSubtarget<PPCSubtarget>();
     if (!STI.hasVSX())
       return false;
 
     bool Changed = false;
     initialize(MF);