[PATCH] D145299: [InstCombine] Generate better code for std::bit_ceil

[Kazu Hirata via Phabricator via llvm-commits]

kazu marked 6 inline comments as done.
kazu added a comment.

Please take a look.  Thanks!



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:3193
+      match(CtlzOp, m_Not(m_Value(X)))) {
+    // We'll stop following the def-use chain when we encounter X.
+  }
----------------
nikic wrote:
> kazu wrote:
> > RKSimon wrote:
> > > empty clause?
> > I would like to find `X` here and leave it `nullptr` if no match occurs.  I don't have any actions when the `if` condition is true.  Should I say something like this?
> > 
> > ```
> >   (void) match(CtlzOp, m_Add(m_Value(X), m_ConstantInt())) ||
> >       match(CtlzOp, m_Sub(m_ConstantInt(), m_Value(X))) ||
> >       match(CtlzOp, m_Not(m_Value(X)));
> > ```
> > 
> Something that I don't really get is why we have to do this separately. Wouldn't something like this be sufficient?
> ```
> Value *RangeOp = Cond0;
> ConstantRange CR = ConstantRange::makeExactICmpRegion(
>     CmpInst::getInversePredicate(Pred), *Cond1);
> 
> if (CtlzOp != RangeOp) {
>   if (match(CtlzOp, m_Add(m_Specific(RangeOp), m_APInt(C)))) {
>     CR = CR.add(*C);
>   }
>   // ...
> }
> ```
I've revamped the matching logic.  No more `RangeOp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145299