[PATCH] D78291: [NFC][DAGCombine] Adding three helper functions and change the getNegatedExpression to negateExpression

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang marked an inline comment as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12693
+  SDValue NegN0 =
+      TLI.getCheaperNegatedExpression(N0, DAG, LegalOperations, ForCodeSize);
+  SDValue NegN1 =
----------------
This lines of code is a bit tricky, we can write it as
```
  TargetLowering::NegatibleCost CostN0 =
      TargetLowering::NegatibleCost::Expensive;
  TargetLowering::NegatibleCost CostN1 =
      TargetLowering::NegatibleCost::Expensive;
  SDValue NegN0 =
      TLI.getNegatedExpression(N0, DAG, LegalOperations, ForCodeSize, CostN0);
  SDValue NegN1 =
      TLI.getNegatedExpression(N1, DAG, LegalOperations, ForCodeSize, CostN1);
  if (NegN0 && NegN1 &&
      (CostN0 == TargetLowering::NegatibleCost::Cheaper ||
       CostN1 == TargetLowering::NegatibleCost::Cheaper)) {
    return DAG.getNode(ISD::FMUL, DL, VT, NegN0, NegN1, Flags);
  }
```
However, they are not semantics the same, as current implementation don't want to negate the expression if the cost of negate N0 and N1 are both neutral while the above implementation did. Technical speaking, it won't have bad impact as we don't change the FMUL if both are neutral. However, it changes the order of the node to combine, which change the PowerPC test llvm/test/CodeGen/PowerPC/qpx-recipest.ll we see in D77319  Maybe, we need another patch to change the semantics and PowerPC test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78291