[PATCH] D62629: [InstCombine] Update fptrunc (fneg x)) -> (fneg (fptrunc x) for unary FNeg
Cameron McInally via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 29 16:39:00 PDT 2019
cameron.mcinally marked an inline comment as not done.
cameron.mcinally added inline comments.
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1625
+ Value *InnerTrunc = Builder.CreateFPTrunc(X, Ty);
+ return BinaryOperator::CreateFNegFMF(InnerTrunc, cast<Instruction>(Op));
> craig.topper wrote:
> > If you started with a unary fneg, shouldn't we end up with a unary fneg?
> So this combine actually covers both unary and binary FNeg. `m_FNeg(X)` will match an `FSub(-0.0,X)`. The end goal is that both will end up producing an unary FNeg.
> But, yeah, I see what you're saying. I could dispatch on the operator type for now, but it would be code to clean up later. I suppose the BinaryOperator will have to be switched to an UnaryOperator in the future anyway, so I'm not sure how much work I'm saving.
> Just thinking aloud, I don't have a preference...
Thinking about this some more, you’re correct. It’s not safe to turn a unary FNeg into a binary FNeg.
I was thinking about this as a short term solution, but we should play it safe in case this does not get updated.
I’ll update the patch...
CHANGES SINCE LAST ACTION
More information about the llvm-commits