[PATCH] D96703: [DAG] foldSubToUSubSat - fold sub(a,trunc(umin(zext(a),b))) -> usubsat(a,trunc(umin(b,c)))

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3156
   RHS = DAG.getNode(ISD::UMIN, DL, SrcVT, RHS, SatLimit);
   RHS = DAG.getZExtOrTrunc(RHS, DL, DstVT);
   LHS = DAG.getZExtOrTrunc(LHS, DL, DstVT);
----------------
craig.topper wrote:
> Is this always a truncate and never a ZExt?
Its always a truncate - its me being lazy (and I think an older implementation used to overflow 80col line). I'll change it as a NFC.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3192
 
+  // sub(a,trunc(umin(zext(a),b))) -> usubsat(a,trunc(umin(b,c)))
+  if (Op1.getOpcode() == ISD::TRUNCATE &&
----------------
craig.topper wrote:
> craig.topper wrote:
> > What is 'c' here? It doesn't appear in the sub(a,trunc(umin(zext(a),b))) pattern
> Or is 'c' a constant which I think we usually write as capital C?
Yes - its a constant - do you think "C" or "SatLimit" would be better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96703