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

Qing Shan Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 16 05:03:28 PDT 2020


steven.zhang created this revision.
steven.zhang added reviewers: RKSimon, spatel, efriedma, arsenm, jsji, PowerPC.
Herald added subscribers: wuzish, hiraditya, wdng.
Herald added a project: LLVM.
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.


This is a NFC patch for D77319 <https://reviews.llvm.org/D77319>. The idea is to hide the getNegatibleCost inside the getNegatedExpression() to have it return null if the cost is expensive, and add some helper function for easy to use. And rename the old getNegatedExpression to negateExpression to avoid the semantic conflict. In the latter patch, we will remove the getNegatibleCost and negateExpression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78291

Files:
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78291.258016.patch
Type: text/x-patch
Size: 21876 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200416/e6fa3c36/attachment.bin>


More information about the llvm-commits mailing list