[PATCH] D138814: [InstCombine] Combine lshr of add -> (a + b < a)

Tue Jan 10 02:06:57 PST 2023

foad added a comment.

In D138814#3973599 <https://reviews.llvm.org/D138814#3973599>, @nikic wrote:

> FWIW our historical stance has always been that uadd.with.overflow is non-canonical, and the canonical pattern is `a + b < a` (for non-constant `b`). uadd.with.overflow generally has worse optimization support, which is why we only form it during CGP for backend purposes.

What's the historical stance on sadd.with.overflow?

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:848
+//      - The add is only used by the shr, or by iK (or narrower) truncates.
+//      - The lshr type has more than 2 bits (other types are boolean math).
+//      - K > 1
----------------
Likewise I don't think you need this line and the "boolean math" comment belongs on the "K > 1" line.

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:860
+
+  if (Ty->getScalarSizeInBits() < 3)
+    return nullptr;
----------------
I don't think you need this check since you check below that K != 1 and it matches the width of X and Y, and the shift type must be strictly wider than that. But I guess it's harmless.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138814