[PATCH] D151358: [LegalizeTypes] Improve expansion of wide SMIN/SMAX/UMIN/UMAX

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I think any of the following makes the new expansion cheaper:

- A subtract-with-borrow/carry instruction
- A cheap setcc/select
- Some cheap way to do a "select" on flags.
- Branches are reasonably cheap, so you can expand the select-of-select construct to branches.

If your target has none of those, a target lowering hook is reasonable.  Seems a little surprising to me, though; all targets I know of with a dedicated "flags" register have a subtract-with-borrow.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2994
+  // high halves of the operands.  Expand this way if it appears profitable.
+  if (RHSVal && (N->getOpcode() == ISD::UMIN || N->getOpcode() == ISD::UMAX) &&
+                 (RHSVal->countLeadingOnes() >= NumHalfBits ||
----------------
bjope wrote:
> It is a bit unclear to me if this just adds heuristics ("it it appears profitable"), or if something inside the if actually is guarded by the conditions in this if.
> 
> To be more specific, if I still want to expand like this for a downstream target, can I just change it to
> 
> ```
> // Heuristic to decide if it is profitable to 
> bool IsProfitable =
>   (RHSVal && (N->getOpcode() == ISD::UMIN || N->getOpcode() == ISD::UMAX) &&
>                  (RHSVal->countLeadingOnes() >= NumHalfBits ||
>                   RHSVal->countLeadingZeros() >= NumHalfBits);
> if (IsProfitable || TLI->avoidSetccInMinMaxExpand()) {
>   ...
> }
> ```
The expansion is fully general; you could use it for anything.  The guard is just checking for profitability (whether the upper umin/umax will simplify).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151358