[PATCH] D94532: [DAG] SimplifyDemandedBits - use KnownBits comparisons to remove ISD::UMIN/UMAX ops

Fri Jan 15 05:35:06 PST 2021

foad added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:1745
+    Known = KnownBits::umin(Known0, Known1);
+    if (Optional<bool> IsULE = KnownBits::ule(Known0, Known1))
+      return TLO.CombineTo(Op, IsULE.getValue() ? Op0 : Op1);
----------------
RKSimon wrote:
> foad wrote:
> > I think this check can miss some opportunities because it is slightly asymmetrical. For example, with 4-bit numbers, if LHS = 110x and RHS = x100 then ule(LHS, RHS) will return None. However ult(LHS, RLHS) would return False, so you could optimise to RHS.
> > 
> > It does seem a shame to have to call both ule and ult just to catch this case. I wonder if there is a better way to code it.
> Yeah, it seems a shame but we don't have good alternatives - adding the ult/ugt test as well doesn't improve any additional existing tests - I can add coverage if you want though?
I'd appreciate it but I certainly won't insist. I think the current patch is fine as far as it goes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94532