[PATCH] D65399: [InstCombine] canonicalize fneg before fmul/fdiv

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: cameron.mcinally, hfinkel, mcberg2017.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.

I'm proposing to reverse the canonicalization of fneg relative to fmul/fdiv. That makes it easier to implement the transforms (and possibly other fneg transforms) in 1 place because we can always start the pattern match from fneg (either the legacy binop or the new unop).

There's a secondary practical benefit seen in PR21914 and PR42681:
https://bugs.llvm.org/show_bug.cgi?id=21914
https://bugs.llvm.org/show_bug.cgi?id=42681
...hoisting fneg rather than sinking seems to play nicer with LICM in IR (although this change may expose analysis holes in the other direction).

1. The instcombine test changes show the expected neutral IR diffs from reversing the order.

2. The reassociation tests show that we were missing an optimization opportunity to fold away fneg-of-fneg. My reading of IEEE-754 says that all of these transforms are allowed (regardless of binop/unop fneg version) because:

"For all other operations [besides copy/abs/negate/copysign], this standard does not specify the sign bit of a NaN result."
In all of these transforms, we always have some other binop (fadd/fsub/fmul/fdiv), so we are free to flip the sign bit of a potential intermediate NaN operand.
(If that interpretation is wrong, then we must already have a bug in the existing transforms?)

3. The clang tests shouldn't exist as-is, but that's effectively a revert of rL367149 <https://reviews.llvm.org/rL367149> (the test broke with an extension of the pre-existing fneg canonicalization in rL367146 <https://reviews.llvm.org/rL367146>).


https://reviews.llvm.org/D65399

Files:
  clang/test/CodeGen/complex-math.c
  llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  llvm/test/Transforms/InstCombine/fadd.ll
  llvm/test/Transforms/InstCombine/fdiv.ll
  llvm/test/Transforms/InstCombine/fmul.ll
  llvm/test/Transforms/InstCombine/fsub.ll
  llvm/test/Transforms/Reassociate/fast-basictest.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65399.212169.patch
Type: text/x-patch
Size: 17703 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190729/ab8bde46/attachment-0001.bin>