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

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Wed May 30 17:02:08 PDT 2018


https://reviews.llvm.org/rL333610

On Mon, May 28, 2018 at 6:26 AM, Sanjay Patel <spatel at rotateright.com>
wrote:

> Thanks. I’m away from a dev machine currently, but I’ll have this fixed by
> the end of the week, if nobody else gets to it first. IIUC, we can reduce
> the test to just one IR instruction that includes a constant expression.
>
> On Sun, May 27, 2018 at 11:20 PM Mikael Holmén <mikael.holmen at ericsson.com>
> wrote:
>
>> I wrote a PR about it:
>>   https://bugs.llvm.org/show_bug.cgi?id=37605
>>
>> Regards,
>> Mikael
>>
>> On 05/25/2018 03:16 PM, Mikael Holmén via llvm-commits wrote:
>> > 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
>> >>
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180530/24b46ace/attachment.html>


More information about the llvm-commits mailing list