[llvm] r330126 - [InstCombine] simplify fneg+fadd folds; NFC

Mikael Holmén via llvm-commits llvm-commits at lists.llvm.org
Fri May 25 06:16:19 PDT 2018


Hi Sanjay,

It seems that with this change instcombine can't decide if it prefers 
fadd or fsub and then ping-pongs for a case we found when compiling 
csmith-generated code.

Reproduce with
  opt -S -o - tr16071.ll -instcombine

With -debug we see

INSTCOMBINE ITERATION #1 on test
IC: DCE:   %1 = select i1 %tobool, i1 true, i1 undef
IC: ADDING: 5 instrs to worklist
IC: Visiting:   %0 = load i16, i16* @c, align 1
IC: Visiting:   %conv = sitofp i16 %0 to float
IC: Visiting:   %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b 
to i32) to float)
IC: Old =   %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to 
i32) to float)
     New =   <badref> = fadd float %conv, fsub (float -0.000000e+00, 
float bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: ADD:   %sub = fadd float %conv, fsub (float -0.000000e+00, float 
bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: ERASE   %1 = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to 
i32) to float)
IC: ADD:   %conv = sitofp i16 %0 to float
IC: Visiting:   %conv = sitofp i16 %0 to float
IC: Visiting:   %sub = fadd float %conv, fsub (float -0.000000e+00, 
float bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: Old =   %sub = fadd float %conv, fsub (float -0.000000e+00, float 
bitcast (i32 ptrtoint (i16* @b to i32) to float))
     New =   <badref> = fsub float %conv, bitcast (i32 ptrtoint (i16* @b 
to i32) to float)
IC: ADD:   %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to 
i32) to float)
IC: ERASE   %1 = fadd float %conv, fsub (float -0.000000e+00, float 
bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: ADD:   %conv = sitofp i16 %0 to float
IC: Visiting:   %conv = sitofp i16 %0 to float
IC: Visiting:   %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b 
to i32) to float)
IC: Old =   %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to 
i32) to float)
     New =   <badref> = fadd float %conv, fsub (float -0.000000e+00, 
float bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: ADD:   %sub = fadd float %conv, fsub (float -0.000000e+00, float 
bitcast (i32 ptrtoint (i16* @b to i32) to float))
IC: ERASE   %1 = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to 
i32) to float)
IC: ADD:   %conv = sitofp i16 %0 to float
[...]

Regards,
Mikael

On 04/16/2018 04:13 PM, Sanjay Patel via llvm-commits wrote:
> Author: spatel
> Date: Mon Apr 16 07:13:57 2018
> New Revision: 330126
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=330126&view=rev
> Log:
> [InstCombine] simplify fneg+fadd folds; NFC
> 
> Two cleanups:
> 1. As noted in D45453, we had tests that don't need FMF that were misplaced in the 'fast-math.ll' test file.
> 2. This removes the final uses of dyn_castFNegVal, so that can be deleted. We use 'match' now.
> 
> Added:
>      llvm/trunk/test/Transforms/InstCombine/fadd.ll
> Modified:
>      llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>      llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
>      llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>      llvm/trunk/test/Transforms/InstCombine/fast-math.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=330126&r1=330125&r2=330126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Mon Apr 16 07:13:57 2018
> @@ -1308,14 +1308,13 @@ Instruction *InstCombiner::visitFAdd(Bin
>     if (Instruction *FoldedFAdd = foldBinOpIntoSelectOrPhi(I))
>       return FoldedFAdd;
>   
> -  // -A + B  -->  B - A
> -  if (Value *LHSV = dyn_castFNegVal(LHS))
> -    return BinaryOperator::CreateFSubFMF(RHS, LHSV, &I);
> -
> -  // A + -B  -->  A - B
> -  if (!isa<Constant>(RHS))
> -    if (Value *V = dyn_castFNegVal(RHS))
> -      return BinaryOperator::CreateFSubFMF(LHS, V, &I);
> +  Value *X;
> +  // (-X) + Y --> Y - X
> +  if (match(LHS, m_FNeg(m_Value(X))))
> +    return BinaryOperator::CreateFSubFMF(RHS, X, &I);
> +  // Y + (-X) --> Y - X
> +  if (match(RHS, m_FNeg(m_Value(X))))
> +    return BinaryOperator::CreateFSubFMF(LHS, X, &I);
>   
>     // Check for (fadd double (sitofp x), y), see if we can merge this into an
>     // integer add followed by a promotion.
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h?rev=330126&r1=330125&r2=330126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h Mon Apr 16 07:13:57 2018
> @@ -376,7 +376,6 @@ private:
>     bool shouldChangeType(unsigned FromBitWidth, unsigned ToBitWidth) const;
>     bool shouldChangeType(Type *From, Type *To) const;
>     Value *dyn_castNegVal(Value *V) const;
> -  Value *dyn_castFNegVal(Value *V, bool NoSignedZero = false) const;
>     Type *FindElementAtOffset(PointerType *PtrTy, int64_t Offset,
>                               SmallVectorImpl<Value *> &NewIndices);
>   
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=330126&r1=330125&r2=330126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Mon Apr 16 07:13:57 2018
> @@ -790,23 +790,6 @@ Value *InstCombiner::dyn_castNegVal(Valu
>     return nullptr;
>   }
>   
> -/// Given a 'fsub' instruction, return the RHS of the instruction if the LHS is
> -/// a constant negative zero (which is the 'negate' form).
> -Value *InstCombiner::dyn_castFNegVal(Value *V, bool IgnoreZeroSign) const {
> -  if (BinaryOperator::isFNeg(V, IgnoreZeroSign))
> -    return BinaryOperator::getFNegArgument(V);
> -
> -  // Constants can be considered to be negated values if they can be folded.
> -  if (ConstantFP *C = dyn_cast<ConstantFP>(V))
> -    return ConstantExpr::getFNeg(C);
> -
> -  if (ConstantDataVector *C = dyn_cast<ConstantDataVector>(V))
> -    if (C->getType()->getElementType()->isFloatingPointTy())
> -      return ConstantExpr::getFNeg(C);
> -
> -  return nullptr;
> -}
> -
>   static Value *foldOperationIntoSelectOperand(Instruction &I, Value *SO,
>                                                InstCombiner::BuilderTy &Builder) {
>     if (auto *Cast = dyn_cast<CastInst>(&I))
> 
> Added: llvm/trunk/test/Transforms/InstCombine/fadd.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fadd.ll?rev=330126&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/fadd.ll (added)
> +++ llvm/trunk/test/Transforms/InstCombine/fadd.ll Mon Apr 16 07:13:57 2018
> @@ -0,0 +1,27 @@
> +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
> +; RUN: opt < %s -instcombine -S | FileCheck %s
> +
> +; -x + y => y - x
> +
> +define float @fneg_op0(float %x, float %y) {
> +; CHECK-LABEL: @fneg_op0(
> +; CHECK-NEXT:    [[ADD:%.*]] = fsub float [[Y:%.*]], [[X:%.*]]
> +; CHECK-NEXT:    ret float [[ADD]]
> +;
> +  %neg = fsub float -0.0, %x
> +  %add = fadd float %neg, %y
> +  ret float %add
> +}
> +
> +; x + -y => x - y
> +
> +define float @fneg_op1(float %x, float %y) {
> +; CHECK-LABEL: @fneg_op1(
> +; CHECK-NEXT:    [[ADD:%.*]] = fsub float [[X:%.*]], [[Y:%.*]]
> +; CHECK-NEXT:    ret float [[ADD]]
> +;
> +  %neg = fsub float -0.0, %y
> +  %add = fadd float %x, %neg
> +  ret float %add
> +}
> +
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/fast-math.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fast-math.ll?rev=330126&r1=330125&r2=330126&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/fast-math.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/fast-math.ll Mon Apr 16 07:13:57 2018
> @@ -406,30 +406,6 @@ define float @fold13_reassoc(float %x) {
>     ret float %sub
>   }
>   
> -; -x + y => y - x
> -; This is always safe.  No FMF required.
> -define float @fold14(float %x, float %y) {
> -; CHECK-LABEL: @fold14(
> -; CHECK-NEXT:    [[ADD:%.*]] = fsub float [[Y:%.*]], [[X:%.*]]
> -; CHECK-NEXT:    ret float [[ADD]]
> -;
> -  %neg = fsub float -0.0, %x
> -  %add = fadd float %neg, %y
> -  ret float %add
> -}
> -
> -; x + -y => x - y
> -; This is always safe.  No FMF required.
> -define float @fold15(float %x, float %y) {
> -; CHECK-LABEL: @fold15(
> -; CHECK-NEXT:    [[ADD:%.*]] = fsub float [[X:%.*]], [[Y:%.*]]
> -; CHECK-NEXT:    ret float [[ADD]]
> -;
> -  %neg = fsub float -0.0, %y
> -  %add = fadd float %x, %neg
> -  ret float %add
> -}
> -
>   ; (select X+Y, X-Y) => X + (select Y, -Y)
>   ; This is always safe.  No FMF required.
>   define float @fold16(float %x, float %y) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
-------------- next part --------------

@b = external global i16, align 1
@c = external global i16, align 1

define void @test() {
entry:
  %0 = load i16, i16* @c, align 1
  %conv = sitofp i16 %0 to float
  %sub = fsub float %conv, bitcast (i32 ptrtoint (i16* @b to i32) to float)
  %tobool = fcmp une float %sub, 0.000000e+00
  %1 = select i1 %tobool, i1 true, i1 undef
  ret void
}


More information about the llvm-commits mailing list