[InstSimplify] Run constant folding if no other simplifications were possible

[igor-laevsky]

Summary

This came in as an unexpected requirement for the D40650.

Before this change we were doing constant folding only for the unrecognised instructions. For the recognised ones we were calling specific constant fold functions from their respective simplification functions. However this specific constant folding functions are slightly weaker that the general ConstantFoldInstruction because they don't fold operands. Turns out that for the insertelement we need full power of the ConstantFoldInstruction or we are going to fail some of the lit tests.

It felt like running constant folding after all InstSimplify transformations is more direct solution rather then trying to include constant folding into each of the simplification functions. Although this mostly affects insertelement case but we can improve later simply by removing constant folding from the simplification functions. This will give us better folding with less effort.

[craig.topper]

Have you checked for compile time impact?

[igor-laevsky]

No, I haven't. I wouldn't expect big compile time impact, but I will run CTMark to be sure.

[spatel]

This seems like it would call ConstantFoldXXX a 2nd time for most instructions?

I'd prefer to just add ConstantFoldInsertElementInstruction() to the top of SimplifyInsertElementInst() to keep things symmetrical for now. If this patch makes sense independent of that, then it could be a follow-up?

[igor-laevsky]

Hi Sanjay,

Adding ConstantFoldInsertElementInstruction doesn't solve the problem I encountered after D40650. For the test Transforms/InstSimplify/pr28725.ll we need to run full ConstantFoldInstruction because it additionally folds instruction operands.

This change indeed causes constant folding to be called twice in some cases. But it's not a total waste - second time we will fold instruction operands and potentially simplify more. In the end I would expect that we will be able to remove all specific constant fold invocations in favour of this single call (providing it will not impact compile time).

[igor-laevsky]

I had run CTMarks and I see total compile time slowdown of about 5% with various benchmarks regressed up to 10%. This does seem a lot especially considering unclear benefit from this change. I need to figure out better approach here.

[spatel]

If you add the usual constant folding check to the top of SimplifyInsertElementInst(), the test in pr28725 will simplify to a constant expression:

define <2 x i16> @test1() {
entry:

ret <2 x i16> <i16 extractvalue (%S select (i1 icmp eq (i16 extractelement (<2 x i16> bitcast (<1 x i32> <i32 1> to <2 x i16>), i32 0), i16 0), %S zeroinitializer, %S { i16 0, i32 1 }), 0), i16 0>

}

This should be fine. InstCombine will turn that into a '0' later.

lib/Analysis/InstructionSimplify.cpp

 }
 
 /// See if we can compute a simplified version of this instruction.
 /// If not, this returns null.
 
 Value *llvm::SimplifyInstruction(Instruction *I, const SimplifyQuery &SQ,
                                  OptimizationRemarkEmitter *ORE) {
   const SimplifyQuery Q = SQ.CxtI ? SQ : SQ.getWithInstruction(I);
-  Value *Result;
+  Value *Result = nullptr;
 
   switch (I->getOpcode()) {
-  default:
-    Result = ConstantFoldInstruction(I, Q.DL, Q.TLI);
-    break;
   case Instruction::FAdd:
     Result = SimplifyFAddInst(I->getOperand(0), I->getOperand(1),
                               I->getFastMathFlags(), Q);
     break;
   case Instruction::Add:
     Result = SimplifyAddInst(I->getOperand(0), I->getOperand(1),
                              cast<BinaryOperator>(I)->hasNoSignedWrap(),
                              cast<BinaryOperator>(I)->hasNoUnsignedWrap(), Q);
Result =
         SimplifyCastInst(I->getOpcode(), I->getOperand(0), I->getType(), Q);
     break;
   case Instruction::Alloca:
     // No simplifications for Alloca and it can't be constant folded.
     Result = nullptr;
     break;
   }
 
+  // Try to do constant folding if above simplifications had failed
+  if (!Result)
+    Result = ConstantFoldInstruction(I, Q.DL, Q.TLI);
+
   // In general, it is possible for computeKnownBits to determine all bits in a
   // value even when the operands are not all constants.
   if (!Result && I->getType()->isIntOrIntVectorTy()) {
     KnownBits Known = computeKnownBits(I, Q.DL, /*Depth*/ 0, Q.AC, I, Q.DT, ORE);
     if (Known.isConstant())
       Result = ConstantInt::get(I->getType(), Known.getConstant());
   }