[PATCH] D140326: DAG: Use getNegatedExpression in combineMinNumMaxNum

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 06:18:27 PST 2023


RKSimon added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4100
+        getNegatedExpression(Op, DAG, LegalOps, OptForSize, Cost, Depth);
+    if (Neg &&
+        (Cost == NegatibleCost::Cheaper || Cost == NegatibleCost::Neutral))
----------------
(style) either create a if (Neg) {} outer wrapper or do a if (!Neg) return SDValue(); early-out


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:392
+                                SDValue RHS, SDValue True, SDValue False,
+                                ISD::CondCode CC, SelectionDAG &DAG);
+
----------------
drop DAG?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10366
+                                         SDValue False, ISD::CondCode CC,
+                                         SelectionDAG &DAG) {
   if ((LHS == True && RHS == False) || (LHS == False && RHS == True))
----------------
Might be worth converting the static method to DAGCombiner::combineMinNumMaxNum as a NFC pre-commit.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10398
+
       if (Combined)
         return DAG.getNode(ISD::FNEG, DL, VT, Combined);
----------------
Move this into the braces above? Its looks that's the only place Combined could be set


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

https://reviews.llvm.org/D140326



More information about the llvm-commits mailing list