[llvm-commits] Patch: Floating point optimizations for SimplifyInstruction

[Michael Ilseman]

On Dec 7, 2012, at 8:49 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Michael,
> 
> On 06/12/12 22:28, Michael Ilseman wrote:
>> Here's two patches to follow the prior two. The first one removes the optimizations that are made redundant in InstCombine, and has InstCombine call the Simplify helpers for FAdd/FSub/FMul just like all the other parts of InstCombine. The second updates SimplifyBinOp to call the new helpers (using default, or zero, fast-math flags).
> 
>> --- a/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>> +++ b/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>> @@ -351,18 +351,8 @@ Instruction *InstCombiner::visitFAdd(BinaryOperator &I) {
>>   bool Changed = SimplifyAssociativeOrCommutative(I);
>>   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
>> 
>> -  if (Constant *RHSC = dyn_cast<Constant>(RHS)) {
>> -    // X + 0 --> X
>> -    if (ConstantFP *CFP = dyn_cast<ConstantFP>(RHSC)) {
>> -      if (CFP->isExactlyValue(ConstantFP::getNegativeZero
>> -                              (I.getType())->getValueAPF()))
>> -        return ReplaceInstUsesWith(I, LHS);
>> -    }
>> -
>> -    if (isa<PHINode>(LHS))
>> -      if (Instruction *NV = FoldOpIntoPhi(I))
>> -        return NV;
> 
> You seem to have dropped this FoldOpIntoPhi transform.  Is that because
> instsimplify will get it anyway?
> 

Dropping that transform was a mistake, I'll keep that in.

>> -  }
>> +  if (Value *V = SimplifyFAddInst(LHS, RHS, I.getFastMathFlags(), TD))
>> +    return ReplaceInstUsesWith(I, V);
>> 
>>   // -A + B  -->  B - A
>>   // -A + -B  -->  -(A + B)
> 
> 
>> --- a/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>> +++ b/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>> @@ -296,20 +296,11 @@ Instruction *InstCombiner::visitFMul(BinaryOperator &I) {
>>   bool Changed = SimplifyAssociativeOrCommutative(I);
>>   Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
>> 
>> -  // Simplify mul instructions with a constant RHS.
>> -  if (Constant *Op1C = dyn_cast<Constant>(Op1)) {
>> -    if (ConstantFP *Op1F = dyn_cast<ConstantFP>(Op1C)) {
>> -      // "In IEEE floating point, x*1 is not equivalent to x for nans.  However,
>> -      // ANSI says we can drop signals, so we can do this anyway." (from GCC)
>> -      if (Op1F->isExactlyValue(1.0))
> 
> Can the pattern matching logic you added in your previous patch use
> isExactlyValue as a cheaper way to test for 1.0?

isExactlyValue does exactly the code from the pattern matching code around the contained APFloat. There doesn't seem to be an easy way out of it.

>From Constant.h:

  bool isExactlyValue(double V) const {
    bool ignored;
    APFloat FV(V);
    FV.convert(Val.getSemantics(), APFloat::rmNearestTiesToEven, &ignored);
    return isExactlyValue(FV);
  }


> 
>> -        return ReplaceInstUsesWith(I, Op0);  // Eliminate 'fmul double %X, 1.0'
>> -    } else if (ConstantDataVector *Op1V = dyn_cast<ConstantDataVector>(Op1C)) {
>> -      // As above, vector X*splat(1.0) -> X in all defined cases.
>> -      if (ConstantFP *F = dyn_cast_or_null<ConstantFP>(Op1V->getSplatValue()))
>> -        if (F->isExactlyValue(1.0))
>> -          return ReplaceInstUsesWith(I, Op0);
>> -    }
>> +  if (Value *V = SimplifyFMulInst(Op0, Op1, I.getFastMathFlags(), TD))
>> +    return ReplaceInstUsesWith(I, V);
>> 
>> +  // Simplify mul instructions with a constant RHS.
>> +  if (isa<Constant>(Op1)) {
>>     // Try to fold constant mul into select arguments.
>>     if (SelectInst *SI = dyn_cast<SelectInst>(Op0))
>>       if (Instruction *R = FoldOpIntoSelect(I, SI))
> 
> Otherwise LGTM.
> 
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits