Rewrite the fsub-0.0 idiom to fneg and always emit fneg for fp negation. This also extends the scalarization cost in instcombine for unary operators to result in the same IR rewrites for fneg as for the idiom.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp | ||
---|---|---|
2137 | 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? |
Thanks for the review!
- rebased
- fixed clang test
- added FIXME for correct DTZ, DAZ handling
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp | ||
---|---|---|
2137 | Added a FIXME for now. |
ping. Is this good to go?
(merge bot failures are due to the m_UnOp syntax, which goes against the coding standard).
Sorry, forgot to submit my nit...
llvm/include/llvm/IR/PatternMatch.h | ||
---|---|---|
75 | Nit: Comment needs updating. |
LGTM - see inline for 1 more comment nit.
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp | ||
---|---|---|
2132–2133 | This comment is stale. Remove or update to something like: // Convert fsub from zero to the canonical fneg instruction. |
Nit: Comment needs updating.