[PATCH] D32596: [DAGCombine] Transform (fadd A, (fmul B, -2.0)) -> (fsub A, (fadd B, B)).

[Chad Rosier via Phabricator via llvm-commits]

mcrosier updated this revision to Diff 97667.
mcrosier retitled this revision from "[DAGCombine] Transform (fmul X, -2.0) --> (fneg (fadd X, X))." to "[DAGCombine] Transform (fadd A, (fmul B, -2.0)) -> (fsub A, (fadd B, B)).".
mcrosier added a comment.
Herald added a subscriber: wdng.

Address reviewers feedback by narrowing transform so that the negation is "free".

@efriedma: This transform now replaces 2 instruction for 2 instruction in the worst case.  For AArch64 it's actually replaces 3 for 2 because we now avoid materializing the -2.0.  Probably true for other targets.
@arsenm: I don't believe this new version causes the instruction encoding to change in size, but please correct me if I'm wrong.
@spatel: This should addresses your comment w.r.t. X86.  If you wish, I can add a target-hook to predicate this transform if the target supports fast multiplication.  Please let me know if that's still a concern.

Geoff and I also discussed this offline.  We also concluded that the previous transform could be bad for ALU bound loops in theory.

Thank you everyone for your feedback.  Very much appreciated.


https://reviews.llvm.org/D32596

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/AArch64/fadd-combines.ll
  test/CodeGen/AMDGPU/fmuladd.f32.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32596.97667.patch
Type: text/x-patch
Size: 4574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170503/d64447cc/attachment.bin>