[PATCH] D104665: [instcombine] Fold overflow check using umulo to comparison

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Seems right- see inline for a few nits along with the clang-format warnings from the bot. 
Please pre-commit the test file with the baseline CHECKs.



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3086
 
+      assert(*EV.idx_begin() == 1);
+
----------------
Add assert msg - "Unexpected extract index for overflow inst"


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3088
+
       // If the normal result of the add is dead, and the RHS is a constant,
+      // we can transform this into a range comparison for many cases.
----------------
This comment is too specific now - we handle more than just adds.
  // If we are only checking for overflow (math result is not used) and the RHS is a constant...


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3105
+        // (unless C == 0, in which case no overflow ever occurs)
+        if (ConstantInt *CI = dyn_cast<ConstantInt>(WO->getRHS())) {
+          assert(!CI->isZero() && "handled by instruction simplify");
----------------
This matches the existing check for add, but we could use match(WO->getRHS(), m_APInt(C)) and handle vector splat constants for almost free. Add a TODO comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104665