[PATCH] D75467: [instcombine] remove fsub to fneg hacks; only emit fneg

Tue Mar 3 01:43:24 PST 2020

simoll marked an inline comment as done.
simoll added inline comments.

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2137
+  if (match(&I, m_FNeg(m_Value(Op))))
+    return UnaryOperator::CreateFNegFMF(Op, &I);

----------------
cameron.mcinally wrote:
> Just noticed that this is a bug -- it needs a guard for denormal flushing. 
> 
> When FTZ or DAZ:
>   fsub -0.0, Denorm ==> +-0
>   fneg Denorm ==> -Denorm
> 
> Handling denormals correctly is an ongoing project. If you'd like to leave this for later, we'll get to it. I see that it's currently broken, so no harm in continuing that way for now.
> 
> Could you add a brief FIXME here?
Added a FIXME for now.
I suppose this should be fixed in PatternMatch.h by making `m_FNeg` refuse to match the fsub idiom for FTZ/DAZ if fneg does not preserve denormal semantics under those settings.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75467/new/

https://reviews.llvm.org/D75467