[PATCH] D148227: [IRCE] Support non-strict range check's predicate

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 17 00:29:01 PDT 2023


mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:306
 
   case ICmpInst::ICMP_SLE:
     std::swap(LHS, RHS);
----------------
This whole code looks like a mess. It's really hard to follow what exactly is it doing. I guess your change should be the same for `UGE`, but from this code structure it's really hard to follow on how it's gonna happen.

I propose the following NFC refactoring:
- If `RHS` is loop-variant, swap `LHS/RHS` and swap predicate;
- all checks are either `IV >(=) const` or `IV <(=) RHS (maybe not const)`. The former are "lower/both checks and latter are "upper/both" checks. 


After that, your change would be: when we have `IV <= RHS`, check that `RHS + 1` doesn not overflow (signed or unsigned overflow - depending on what predicate is used), and if yes, add `1` to `RHS`.

This, at least, would be easier to understand.


================
Comment at: llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:317
+      const SCEV *One = SE.getOne(LHS->getType());
+      const SCEV *L = SE.getSCEV(LHS);
+      if (SE.willNotOverflow(Instruction::BinaryOps::Add, /*Signed*/ true, L,
----------------
Name `L` is almost exclusively used for loops, call this `LHSS`.


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

https://reviews.llvm.org/D148227



More information about the llvm-commits mailing list