[WIP][PowerPC] run additional InstCombine pass after PPCCTRLoop pass

[inouehrs]

Summary

This patch add an additional InstCombine pass executed after PPCCTRLoop pass.
The intention of this additional InstCombine pass is to clean up accesses to induction variables in a loop if PPCCTRLoop pass makes them dead code.
A motivating example is as follow:

bool func(long a[N][N], long row, long col) {
    for (long i = row, j = col; j >= 0 && i < N; i++, j--)
        if (a[i][j]) return false;
    return true;
}

This loop is compiled into

.LBB0_2:                                # %for.body
	ldu 7, 56(6)
	cmpldi	 7, 0
	bne	 0, .LBB0_5
	addi 4, 4, 1
	addi 5, 5, -1
	bdnz .LBB0_2

Two addi for r4 and r5 are not used here.
By running InstCombine (even with ExpensiveCombines = false), these instructions are eliminated.

By adding the last InstCombine pass, 43 test cases (including those not optimized by PPCCTRLoop) fail. So it looks like this last InstCombine pass cleanup the code in many cases.
I observed that adding this pass increases compilation time by about 0.5% (both in # cycles and in # instructions).

I like to hear the opinion on whether adding this last InstCombine pass is reasonable as a trade-off between the compilation cost and code quality before modifying lots of test cases.

[hfinkel]

I'm not worried here about compile time, there's a different problem, however. If you run InstCombine after CGP, it will likely undo things that CGP does (such as sinking things, splitting things, etc. - a bunch of anti-canonical transformations). How much this matters for us I'm not sure, we'd need to audit. If this is just about eliminating dead code, does running something like ADCE take care of it?

It would certainly be interesting to describe what other code-quality improvements have shown up by doing this (in other test cases, etc.).

Regardless, this needs a test case.

[inouehrs]

Hal, thank you so much for the suggestion.
Actually, I realized that a recent patch for PPCCTRLoop (https://reviews.llvm.org/D34829) already resolved this issue.
So I abandon this patch. But I like to investigate what changes the last instcombine make in existing test cases.

lib/Target/PowerPC/LLVMBuild.txt

 has_asmprinter = 1
 has_disassembler = 1
 has_jit = 1
 
 [component_1]
 type = Library
 name = PowerPCCodeGen
 parent = PowerPC
-required_libraries = Analysis AsmPrinter CodeGen Core MC PowerPCAsmPrinter PowerPCDesc PowerPCInfo Scalar SelectionDAG Support Target TransformUtils
+required_libraries = Analysis AsmPrinter CodeGen Core InstCombine MC PowerPCAsmPrinter PowerPCDesc PowerPCInfo Scalar SelectionDAG Support Target TransformUtils
 add_to_library_groups = PowerPC

lib/Target/PowerPC/PPCTargetMachine.cpp

void PPCPassConfig::addIRPasses() {
 
   TargetPassConfig::addIRPasses();
 }
 
 bool PPCPassConfig::addPreISel() {
   if (!DisablePreIncPrep && getOptLevel() != CodeGenOpt::None)
     addPass(createPPCLoopPreIncPrepPass(getPPCTargetMachine()));
 
-  if (!DisableCTRLoops && getOptLevel() != CodeGenOpt::None)
+  if (!DisableCTRLoops && getOptLevel() != CodeGenOpt::None) {
     addPass(createPPCCTRLoops());
 
+    // Run instcombine to eliminate induction variables unused after
+    // rewritten by CTR loop optimization.
+    addPass(createInstructionCombiningPass(/* ExpensiveCombines = */false));
+  }
+
   return false;
 }
 
 bool PPCPassConfig::addILPOpts() {
   addPass(&EarlyIfConverterID);
 
   if (EnableMachineCombinerPass)
     addPass(&MachineCombinerID);