[PATCH] D85548: Allow fneg + strict_fadd -> strict_fsub in DAGCombiner

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12824
+  // fold (strict_fadd A, (fneg B)) -> (strict_fsub A, B)
+  if (!LegalOperations || TLI.isOperationLegalOrCustom(ISD::STRICT_FSUB, VT))
+    if (SDValue NegN1 = TLI.getCheaperNegatedExpression(
----------------
arsenm wrote:
> kpn wrote:
> > arsenm wrote:
> > > There is some strange legality predicate you are supposed to use for the strict cases, although I don't fully understand it
> > Where is this predicate? Is there an existing case that can be modeled after?
> > 
> > My thinking is that this transform is probably safe because the FNeg will never trap and it will never round so it should therefore be safe to fold it into the add/sub. But I'm no expert on floating-point I admit.
> This is totally valid. But for some reason, we have getStrictFPOperationAction which differs from checking the regular legalization action on the STRICT_ opcode
I think getStrictFPOperationAction works along with the code in SelectionDAGISel that mutates strict nodes to non-strict nodes for targets that haven't done the work. It was intended to query whether the target will be able to handle the mutated node. It should go away when all targets have done the work.

Since I think we're now blocking clang from using strict fp on targets that haven't done the work. Then we probably can just use the normal legal operation check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85548