[PATCH] D156811: [InstCombine] Fold `select` of `srem` and conditional add

[Antonio Frighetto via Phabricator via llvm-commits]

antoniofrighetto added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2616
+        Pred == ICmpInst::ICMP_SLT && CmpCond == Rem))
+    return nullptr;
+
----------------
goldstein.w.n wrote:
> antoniofrighetto wrote:
> > goldstein.w.n wrote:
> > > Can you move this check to before the other match. 1) think its clearer, 2) if you add the generic `isKnownToBeAPowerOfTwo` approach is good to exhaust all the cheap checks before the recursive call.
> > Moved above, and moved from using `m_Power2` to the generic `isKnownToBeAPowerOfTwo`.
> Please update you proofs to replace `8` with a variable `C` and use assume to provide its constraints (in your case pow2 or zero).
I had tried with something [[ https://alive2.llvm.org/ce/z/_NfVxt | similar ]], but i128 seems to lead Alive2 to timeout, and there's no assume for vectors. Updated proofs with [[ https://alive2.llvm.org/ce/z/9_KG6c | this ]], if it suffices.


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

https://reviews.llvm.org/D156811