[PATCH] D106139: [DAGCombine] Combine srX of add that intends to get the carry as uaddo

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8743
 
+  if (SDValue Overflow = combineSRXToOverflow(N))
+    return Overflow;
----------------
Will we hit the DAG.SignBitIsZero(N0) case on line 8736 before we get a chance to match this pattern?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8763
+
+  if (Level >= AfterLegalizeVectorOps || N0.getOpcode() != ISD::ADD)
+    return SDValue();
----------------
If we keep this in DAGCombine, we should use `LegalOperations` rather than `Level >= AfterLegalizeVectorOps`


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8818
+
+  DAG.ReplaceAllUsesOfValueWith(N0.getValue(0), UAddO.getValue(0));
+  return DAG.getZExtOrTrunc(UAddO.getValue(1), SDLoc(N), N->getValueType(0));
----------------
N0.getValue(0) and UAddO.getValue(0) don't have the same type do they?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106139