[llvm] r330126 - [InstCombine] simplify fneg+fadd folds; NFC
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Thu May 31 06:35:35 PDT 2018
On 05/31/2018 02:02 AM, Sanjay Patel wrote:
> https://reviews.llvm.org/rL333610 <https://reviews.llvm.org/rL333610>
>
Thanks!
> On Mon, May 28, 2018 at 6:26 AM, Sanjay Patel <spatel at rotateright.com
> <mailto: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 <mailto:mikael.holmen at ericsson.com>> wrote:
>
> I wrote a PR about it:
> https://bugs.llvm.org/show_bug.cgi?id=37605
> <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
> <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
> <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
> <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
> <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
> <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
> <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 <mailto:llvm-commits at lists.llvm.org>
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> >>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> >
>
>
More information about the llvm-commits
mailing list