[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

[Hirochika Matsumoto via Phabricator via llvm-commits]

hkmatsumoto added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915
+/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and
+/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2).
+static Value *foldIsPowerOf2OrZero(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd,
----------------
spatel wrote:
> Why create ">= 2" instead of "> 1" directly? 
> 
> I don't think it makes the transform or code any clearer with ">= 2", and we will always canonicalize to the other form, so I would prefer to go directly to the final result for efficiency.
You're right. I thought it is fine to let another pass do further transformation but in retrospect, folding >= 2 to > 1 is so trivial that we should do it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077