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

[Duncan Sands]

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?

> -  }
> +  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?

> -        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.